[MIPS] Use softPromoteHalf legalization for fp16 rather than PromoteFloat#110199
[MIPS] Use softPromoteHalf legalization for fp16 rather than PromoteFloat#110199
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
2ffa43e to
260ff02
Compare
nikic
left a comment
There was a problem hiding this comment.
Fix #97975.
This only fixes it for the MIPS architecture, so should not close the issue with "Fix".
The fix itself here looks fine to me, but I feel like the test coverage is very weak. There's just that one test, and because it's not a generated one, it omits too much information to even tell if the assembly is correct.
| SDValue Lo = DAG.getNode(MipsISD::Lo, DL, Ty, | ||
| getTargetNode(N, Ty, DAG, LoFlag)); | ||
| SDValue Lo = | ||
| DAG.getNode(MipsISD::Lo, DL, Ty, getTargetNode(N, Ty, DAG, LoFlag)); |
There was a problem hiding this comment.
Unrelated reformat? Please use git-clang-format to format PRs, it should leave unrelated lines alone.
There was a problem hiding this comment.
I had used the git clang-format tool before submitting this PR, and it automatically modified the irrelevant code format, so I submitted it together.
There was a problem hiding this comment.
I would do it locally to check it again.
260ff02 to
a618b49
Compare
I would update the title and enhance the test file. |
a618b49 to
38fcdfa
Compare
38fcdfa to
3e5c16f
Compare
|
Ping |
|
Is it possible to add a test with |
3e5c16f to
a87fac2
Compare
|
@tgross35 Done. |
…onger cause a crash (#116569) This PR fixes a bug introduced by #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) #2 0x000055a3a40144a8 SignalHandler(int) Signals.cpp:0:0 #3 0x00007f00bde558c0 __restore_rt libc_sigaction.c:0:0 #4 0x00007f00bdea462c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76 #5 0x00007f00bde55822 gsignal ./signal/../sysdeps/posix/raise.c:27:6 #6 0x00007f00bde3e4af abort ./stdlib/abort.c:81:7 #7 0x000055a3a3f80e3c llvm::report_fatal_error(llvm::Twine const&, bool) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x323de3c) #8 0x000055a3a2e20dfa (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x20dddfa) #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 #10 0x000055a3a3d896a9 llvm::SelectionDAGISel::LowerArguments(llvm::Function const&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30466a9) #11 0x000055a3a3e0b3ec llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c83ec) #12 0x000055a3a3e09e21 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c6e21) #13 0x000055a3a2aae1ca llvm::MipsDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) MipsISelDAGToDAG.cpp:0:0 #14 0x000055a3a3e07706 llvm::SelectionDAGISelLegacy::runOnMachineFunction(llvm::MachineFunction&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c4706) #15 0x000055a3a3051ed6 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x230eed6) #16 0x000055a3a35a3ec9 llvm::FPPassManager::runOnFunction(llvm::Function&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x2860ec9) #17 0x000055a3a35ac3b2 llvm::FPPassManager::runOnModule(llvm::Module&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x28693b2) #18 0x000055a3a35a499c llvm::legacy::PassManagerImpl::run(llvm::Module&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x286199c) #19 0x000055a3a262abbb main (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x18e7bbb) #20 0x00007f00bde3fc4c __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3 #21 0x00007f00bde3fd05 call_init ./csu/../csu/libc-start.c:128:20 #22 0x00007f00bde3fd05 __libc_start_main@GLIBC_2.2.5 ./csu/../csu/libc-start.c:347:5 #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 #110199. This PR implements the first solution to not introduce any more ABI changes as #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.
On PowerPC targets, `half` uses the default legalization of promoting to a `f32`. However, this has some fundamental issues related to inability to round trip. Resolve this by switching to the soft legalization, which passes `f16` as an `i16`. The PowerPC ABI Specification does not define a `_Float16` type, so the calling convention changes are acceptable. Fixes the PowerPC portion of [1]. A similar change was done for MIPS in f0231b6 ("[MIPS] Use softPromoteHalf legalization for fp16 rather than PromoteFloat (llvm#110199)") and for Loongarch in 13280d9 ("[loongarch][DAG][FREEZE] Fix crash when FREEZE a half(f16) type on loongarch (llvm#107791)"). [1]: llvm#97975
On PowerPC targets, `half` uses the default legalization of promoting to a `f32`. However, this has some fundamental issues related to inability to round trip. Resolve this by switching to the soft legalization, which passes `f16` as an `i16`. The PowerPC ABI Specification does not define a `_Float16` type, so the calling convention changes are acceptable. Fixes the PowerPC portion of [1]. A similar change was done for MIPS in f0231b6 ("[MIPS] Use softPromoteHalf legalization for fp16 rather than PromoteFloat (llvm#110199)") and for Loongarch in 13280d9 ("[loongarch][DAG][FREEZE] Fix crash when FREEZE a half(f16) type on loongarch (llvm#107791)"). [1]: llvm#97975
Fix part of #97975.