Skip to content

Recommit "[LoopFlatten] Enable it by default" #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

stuij
Copy link
Owner

@stuij stuij commented Apr 29, 2024

This got reverted by commit 8250180 because of a reported miscompilation in

The enablement of LoopFlatten got reverted by commit 8250180 because of a
reported miscompilation in llvm#59339. That issue got fixed by commit 161bfa5.
@stuij stuij force-pushed the enable-loop-flatten branch from 3a974d9 to de7fe09 Compare April 29, 2024 13:43
@stuij
Copy link
Owner Author

stuij commented Apr 29, 2024

wrong fork :)

@stuij stuij closed this Apr 29, 2024
stuij pushed a commit that referenced this pull request Nov 21, 2024
…tely from Linux (llvm#115722)

This test fails on
https://lab.llvm.org/staging/#/builders/197/builds/76/steps/18/logs/FAIL__lldb-shell__inline_sites_live_cpp
because of a little difference in the lldb output.

```
# .---command stderr------------
# | C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\test\Shell\SymbolFile\NativePDB\inline_sites_live.cpp:25:11: error: CHECK: expected string not found in input
# | // CHECK: * thread #1, stop reason = breakpoint 1
# |           ^
# | <stdin>:1:1: note: scanning from here
# | (lldb) platform select remote-linux
# | ^
# | <stdin>:28:27: note: possible intended match here
# | * thread #1, name = 'inline_sites_li', stop reason = breakpoint 1.3
# |                           ^
# | 

```
stuij pushed a commit that referenced this pull request Nov 21, 2024
Add patterns to fold MOV (scalar, predicated) to MOV (imm, pred,
merging) or MOV (imm, pred, zeroing) as appropriate.

This affects the `@llvm.aarch64.sve.dup` intrinsics, which currently
generate MOV (scalar, predicated) instructions even when the
immediate forms are possible. For example:
```
svuint8_t mov_z_b(svbool_t p) {
  return svdup_u8_z(p, 1);
}
```
Currently generates:
```
mov_z_b(__SVBool_t):
        mov     z0.b, #0
        mov     w8, #1
        mov     z0.b, p0/m, w8
        ret
```
Instead of:
```
mov_z_b(__SVBool_t):
        mov     z0.b, p0/z, #1
        ret
```
stuij pushed a commit that referenced this pull request Nov 21, 2024
…onger cause a crash (llvm#116569)

This PR fixes a bug introduced by llvm#110199, which causes any half float
argument to crash the compiler on MIPS64.

Currently compiling this bit of code with `llc -mtriple=mips64`: 
```
define void @half_args(half %a) nounwind {
entry:
        ret void
}
```

Crashes with the following log:
```
LLVM ERROR: unable to allocate function argument #0
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: llc -mtriple=mips64
1.	Running pass 'Function Pass Manager' on module '<stdin>'.
2.	Running pass 'MIPS DAG->DAG Pattern Instruction Selection' on function '@half_args'
 #0 0x000055a3a4013df8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x32d0df8)
 #1 0x000055a3a401199e llvm::sys::RunSignalHandlers() (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x32ce99e)
 llvm#2 0x000055a3a40144a8 SignalHandler(int) Signals.cpp:0:0
 llvm#3 0x00007f00bde558c0 __restore_rt libc_sigaction.c:0:0
 llvm#4 0x00007f00bdea462c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 llvm#5 0x00007f00bde55822 gsignal ./signal/../sysdeps/posix/raise.c:27:6
 llvm#6 0x00007f00bde3e4af abort ./stdlib/abort.c:81:7
 llvm#7 0x000055a3a3f80e3c llvm::report_fatal_error(llvm::Twine const&, bool) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x323de3c)
 llvm#8 0x000055a3a2e20dfa (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x20dddfa)
 llvm#9 0x000055a3a2a34e20 llvm::MipsTargetLowering::LowerFormalArguments(llvm::SDValue, unsigned int, bool, llvm::SmallVectorImpl<llvm::ISD::InputArg> const&, llvm::SDLoc const&, llvm::SelectionDAG&, llvm::SmallVectorImpl<llvm::SDValue>&) const MipsISelLowering.cpp:0:0
llvm#10 0x000055a3a3d896a9 llvm::SelectionDAGISel::LowerArguments(llvm::Function const&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30466a9)
llvm#11 0x000055a3a3e0b3ec llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c83ec)
llvm#12 0x000055a3a3e09e21 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c6e21)
llvm#13 0x000055a3a2aae1ca llvm::MipsDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) MipsISelDAGToDAG.cpp:0:0
llvm#14 0x000055a3a3e07706 llvm::SelectionDAGISelLegacy::runOnMachineFunction(llvm::MachineFunction&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c4706)
llvm#15 0x000055a3a3051ed6 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x230eed6)
llvm#16 0x000055a3a35a3ec9 llvm::FPPassManager::runOnFunction(llvm::Function&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x2860ec9)
llvm#17 0x000055a3a35ac3b2 llvm::FPPassManager::runOnModule(llvm::Module&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x28693b2)
llvm#18 0x000055a3a35a499c llvm::legacy::PassManagerImpl::run(llvm::Module&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x286199c)
llvm#19 0x000055a3a262abbb main (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x18e7bbb)
llvm#20 0x00007f00bde3fc4c __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
llvm#21 0x00007f00bde3fd05 call_init ./csu/../csu/libc-start.c:128:20
llvm#22 0x00007f00bde3fd05 __libc_start_main@GLIBC_2.2.5 ./csu/../csu/libc-start.c:347:5
llvm#23 0x000055a3a2624921 _start /builddir/glibc-2.39/csu/../sysdeps/x86_64/start.S:117:0
```

This is caused by the fact that after the change, `f16`s are no longer
lowered as `f32`s in calls.

Two possible fixes are available:
- Update calling conventions to properly support passing `f16` as
integers.
- Update `useFPRegsForHalfType()` to return `true` so that `f16` are
still kept in `f32` registers, as before llvm#110199.

This PR implements the first solution to not introduce any more ABI
changes as llvm#110199 already did.

As of what is the correct ABI for halfs, I don't think there is a
correct answer. GCC doesn't support halfs on MIPS, and I couldn't find
any information on old MIPS ABI manuals either.
stuij pushed a commit that referenced this pull request Nov 21, 2024
…lvm#116655)

This is the first part of the effort to make parsing of clause modifiers
more uniform and robust. Currently, when multiple modifiers are allowed,
the parser will expect them to appear in a hard-coded order.
Additionally, modifier properties (such as "ultimate") are checked
separately for each case.

The overall plan is
1. Extract all modifiers into their own top-level classes, and then
equip them with sets of common properties that will allow performing the
property checks generically, without refering to the specific kind of
the modifier.
2. Define a parser (as a separate class) for each modifier.
3. For each clause define a union (std::variant) of all allowable
modifiers, and parse the modifiers as a list of these unions.

The intent is also to isolate parts of the code that could eventually be
auto-generated.

OpenMP modifier overhaul: #1/3
stuij added a commit that referenced this pull request Aug 6, 2025
With this new A320 in-order core, we follow adding the
FeatureUseFixedOverScalableIfEqualCost feature to A510 and A520
(llvm#132246), which reaps the same code generation benefits of preferring fixed
over scalable when the cost is equal.

So when we have:
```
void foo(float* a, float* b, float* dst, unsigned n) {
    for (unsigned i = 0; i < n; ++i)
        dst[i] = a[i] + b[i];
}
```

When compiling without the feature enabled, we get:
```
...
    ld1b    { z0.b }, p0/z, [x0, x10]
    ld1b    { z2.b }, p0/z, [x1, x10]
    add     x12, x0, x10
    ldr     z1, [x12, #1, mul vl]
    add     x12, x1, x10
    ldr     z3, [x12, #1, mul vl]
    fadd    z0.s, z2.s, z0.s
    add     x12, x2, x10
    fadd    z1.s, z3.s, z1.s
    dech    x11
    st1b    { z0.b }, p0, [x2, x10]
    incb    x10, all, mul llvm#2
    str     z1, [x12, #1, mul vl]
...
```

When compiling with, we get:
```
...
  	ldp	    q0, q1, [x12, #-16]
	ldp	    q2, q3, [x11, #-16]
	subs	x13, x13, llvm#8
	fadd	v0.4s, v2.4s, v0.4s
	fadd	v1.4s, v3.4s, v1.4s
	add	    x11, x11, llvm#32
	add	    x12, x12, llvm#32
	stp	    q0, q1, [x10, #-16]
	add	    x10, x10, llvm#32

...
```

This patch also moves FeatureUseFixedOverScalableIfEqualCost for A510 and A520
from the CPU features to the tune features.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant