Skip to content
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

LLVM f128 -> f16 conversion selection failure on powerpc64le #92866

Open
tgross35 opened this issue May 21, 2024 · 11 comments · May be fixed by #97677
Open

LLVM f128 -> f16 conversion selection failure on powerpc64le #92866

tgross35 opened this issue May 21, 2024 · 11 comments · May be fixed by #97677
Assignees
Labels
backend:PowerPC crash Prefer [crash-on-valid] or [crash-on-invalid] llvm:SelectionDAG SelectionDAGISel as well

Comments

@tgross35
Copy link

tgross35 commented May 21, 2024

The below fails to select on target powerpc64le-unknown-linux-gnu:

define half @demo(fp128 %a) unnamed_addr {
start:
  %_0 = fptrunc fp128 %a to half
  ret half %_0
}
LLVM ERROR: Cannot select: 0x8255100: i32 = fp_to_fp16 0x8254d10
  0x8254d10: f128,ch = CopyFromReg 0x81d62a0, Register:f128 %0
    0x8254ca0: f128 = Register %0
In function: demo
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /opt/compiler-explorer/clang-trunk/bin/llc -o /app/output.s -x86-asm-syntax=intel -mtriple=powerpc64le-unknown-linux-gnu <source>
1.	Running pass 'Function Pass Manager' on module '<source>'.
2.	Running pass 'PowerPC DAG->DAG Pattern Instruction Selection' on function '@demo'
 #0 0x00000000036a9bd8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/compiler-explorer/clang-trunk/bin/llc+0x36a9bd8)
 #1 0x00000000036a754c SignalHandler(int) Signals.cpp:0:0
 #2 0x0000798ba1642520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #3 0x0000798ba16969fc pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0x969fc)
 #4 0x0000798ba1642476 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x42476)
 #5 0x0000798ba16287f3 abort (/lib/x86_64-linux-gnu/libc.so.6+0x287f3)
 #6 0x0000000000728eee llvm::UniqueStringSaver::save(llvm::StringRef) (.cold) StringSaver.cpp:0:0
 #7 0x0000000003479844 llvm::SelectionDAGISel::CannotYetSelect(llvm::SDNode*) (/opt/compiler-explorer/clang-trunk/bin/llc+0x3479844)
 #8 0x0000000003480265 llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*, unsigned char const*, unsigned int) (/opt/compiler-explorer/clang-trunk/bin/llc+0x3480265)
 #9 0x0000000001776d27 (anonymous namespace)::PPCDAGToDAGISel::Select(llvm::SDNode*) PPCISelDAGToDAG.cpp:0:0
#10 0x0000000003476d07 llvm::SelectionDAGISel::DoInstructionSelection() (/opt/compiler-explorer/clang-trunk/bin/llc+0x3476d07)
#11 0x0000000003484c65 llvm::SelectionDAGISel::CodeGenAndEmitDAG() (/opt/compiler-explorer/clang-trunk/bin/llc+0x3484c65)
#12 0x0000000003487918 llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (/opt/compiler-explorer/clang-trunk/bin/llc+0x3487918)
#13 0x000000000348a0f9 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (.part.0) SelectionDAGISel.cpp:0:0
#14 0x000000000177d7a7 (anonymous namespace)::PPCDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) PPCISelDAGToDAG.cpp:0:0
#15 0x00000000027da790 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (.part.0) MachineFunctionPass.cpp:0:0
#16 0x0000000002cff152 llvm::FPPassManager::runOnFunction(llvm::Function&) (/opt/compiler-explorer/clang-trunk/bin/llc+0x2cff152)
#17 0x0000000002cff2d1 llvm::FPPassManager::runOnModule(llvm::Module&) (/opt/compiler-explorer/clang-trunk/bin/llc+0x2cff2d1)
#18 0x0000000002d00f80 llvm::legacy::PassManagerImpl::run(llvm::Module&) (/opt/compiler-explorer/clang-trunk/bin/llc+0x2d00f80)
#19 0x000000000083d4c4 compileModule(char**, llvm::LLVMContext&) llc.cpp:0:0
#20 0x000000000073b0ce main (/opt/compiler-explorer/clang-trunk/bin/llc+0x73b0ce)
#21 0x0000798ba1629d90 (/lib/x86_64-linux-gnu/libc.so.6+0x29d90)
#22 0x0000798ba1629e40 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e40)
#23 0x00000000008350fe _start (/opt/compiler-explorer/clang-trunk/bin/llc+0x8350fe)
Program terminated with signal: SIGSEGV
Compiler returned: 139

Link: https://llvm.godbolt.org/z/16W8EKzfa

both powerpc64 and powerpc (32-bit) complete selection and emit a call to __trunctfhf2. I am not sure if this is correct or if it should try to call __trunckfhf2 instead, but all three targets (powerpc64, powerpc64le, powerpc) should all probably act the same.

@tgross35 tgross35 changed the title LLVM f128 -> f16 failure or LLVM f128 -> f16 conversion selection failure on powerpc64le May 21, 2024
@EugeneZelenko EugeneZelenko added backend:PowerPC crash Prefer [crash-on-valid] or [crash-on-invalid] llvm:SelectionDAG SelectionDAGISel as well and removed new issue labels May 21, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 21, 2024

@llvm/issue-subscribers-backend-powerpc

Author: Trevor Gross (tgross35)

The below fails to select on target `powerpc64le-unknown-linux-gnu`:
define half @<!-- -->demo(fp128 %a) unnamed_addr {
start:
  %_0 = fptrunc fp128 %a to half
  ret half %_0
}
LLVM ERROR: Cannot select: 0x8255100: i32 = fp_to_fp16 0x8254d10
  0x8254d10: f128,ch = CopyFromReg 0x81d62a0, Register:f128 %0
    0x8254ca0: f128 = Register %0
In function: demo
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /opt/compiler-explorer/clang-trunk/bin/llc -o /app/output.s -x86-asm-syntax=intel -mtriple=powerpc64le-unknown-linux-gnu &lt;source&gt;
1.	Running pass 'Function Pass Manager' on module '&lt;source&gt;'.
2.	Running pass 'PowerPC DAG-&gt;DAG Pattern Instruction Selection' on function '@<!-- -->demo'
 #<!-- -->0 0x00000000036a9bd8 llvm::sys::PrintStackTrace(llvm::raw_ostream&amp;, int) (/opt/compiler-explorer/clang-trunk/bin/llc+0x36a9bd8)
 #<!-- -->1 0x00000000036a754c SignalHandler(int) Signals.cpp:0:0
 #<!-- -->2 0x0000798ba1642520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #<!-- -->3 0x0000798ba16969fc pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0x969fc)
 #<!-- -->4 0x0000798ba1642476 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x42476)
 #<!-- -->5 0x0000798ba16287f3 abort (/lib/x86_64-linux-gnu/libc.so.6+0x287f3)
 #<!-- -->6 0x0000000000728eee llvm::UniqueStringSaver::save(llvm::StringRef) (.cold) StringSaver.cpp:0:0
 #<!-- -->7 0x0000000003479844 llvm::SelectionDAGISel::CannotYetSelect(llvm::SDNode*) (/opt/compiler-explorer/clang-trunk/bin/llc+0x3479844)
 #<!-- -->8 0x0000000003480265 llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*, unsigned char const*, unsigned int) (/opt/compiler-explorer/clang-trunk/bin/llc+0x3480265)
 #<!-- -->9 0x0000000001776d27 (anonymous namespace)::PPCDAGToDAGISel::Select(llvm::SDNode*) PPCISelDAGToDAG.cpp:0:0
#<!-- -->10 0x0000000003476d07 llvm::SelectionDAGISel::DoInstructionSelection() (/opt/compiler-explorer/clang-trunk/bin/llc+0x3476d07)
#<!-- -->11 0x0000000003484c65 llvm::SelectionDAGISel::CodeGenAndEmitDAG() (/opt/compiler-explorer/clang-trunk/bin/llc+0x3484c65)
#<!-- -->12 0x0000000003487918 llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&amp;) (/opt/compiler-explorer/clang-trunk/bin/llc+0x3487918)
#<!-- -->13 0x000000000348a0f9 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&amp;) (.part.0) SelectionDAGISel.cpp:0:0
#<!-- -->14 0x000000000177d7a7 (anonymous namespace)::PPCDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&amp;) PPCISelDAGToDAG.cpp:0:0
#<!-- -->15 0x00000000027da790 llvm::MachineFunctionPass::runOnFunction(llvm::Function&amp;) (.part.0) MachineFunctionPass.cpp:0:0
#<!-- -->16 0x0000000002cff152 llvm::FPPassManager::runOnFunction(llvm::Function&amp;) (/opt/compiler-explorer/clang-trunk/bin/llc+0x2cff152)
#<!-- -->17 0x0000000002cff2d1 llvm::FPPassManager::runOnModule(llvm::Module&amp;) (/opt/compiler-explorer/clang-trunk/bin/llc+0x2cff2d1)
#<!-- -->18 0x0000000002d00f80 llvm::legacy::PassManagerImpl::run(llvm::Module&amp;) (/opt/compiler-explorer/clang-trunk/bin/llc+0x2d00f80)
#<!-- -->19 0x000000000083d4c4 compileModule(char**, llvm::LLVMContext&amp;) llc.cpp:0:0
#<!-- -->20 0x000000000073b0ce main (/opt/compiler-explorer/clang-trunk/bin/llc+0x73b0ce)
#<!-- -->21 0x0000798ba1629d90 (/lib/x86_64-linux-gnu/libc.so.6+0x29d90)
#<!-- -->22 0x0000798ba1629e40 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e40)
#<!-- -->23 0x00000000008350fe _start (/opt/compiler-explorer/clang-trunk/bin/llc+0x8350fe)
Program terminated with signal: SIGSEGV
Compiler returned: 139

Link: https://llvm.godbolt.org/z/16W8EKzfa

both powerpc64 and powerpc (32-bit) complete selection and emit a call to __trunctfhf2. I am not sure if this is correct or if it should try to call __trunckfhf2 instead, but all three targets (powerpc64, powerpc64le, powerpc) should all probably act the same.

@ecnelises
Copy link
Member

The reverse conversion also crashes:

__float128 from(__fp16 *f) { return *f; }
void to(__fp16 *f, __float128 g) { *f = (__fp16)g; }

For pre-power9 targets, f16->f128 should generate __extendhftf2 and the reverse should generate __trunctfhf2.

For power9 and later targets, f16->f128 should be xscvhpdp+xscvdpqp, the reverse should be xscvqpdp+xscvdphp.

@tgross35
Copy link
Author

Thanks for confirming.

For pre-power9 targets, f16->f128 should generate __extendhftf2 and the reverse should generate __trunctfhf2.

Any reason it uses tf rather than kf naming here?

@ecnelises ecnelises self-assigned this May 30, 2024
@ecnelises ecnelises assigned EsmeYi and unassigned ecnelises Jun 12, 2024
@ecnelises
Copy link
Member

Any reason it uses tf rather than kf naming here?

No idea.. libgcc does not contain __extendhfkf2, need to check functionality of __extendhftf2.

@EsmeYi
Copy link
Contributor

EsmeYi commented Jun 27, 2024

Any reason it uses tf rather than kf naming here?

No idea.. libgcc does not contain __extendhfkf2, need to check functionality of __extendhftf2.

In the patch https://reviews.llvm.org/D64282, it says In libgcc, libgcc/config/rs6000/float128-sed converts TF names to KF names.

@tgross35
Copy link
Author

It looks like extendhf is not included from that file https://github.com/gcc-mirror/gcc/blob/c7cb0dd94589ab501bca27f93641b4074e5a2e99/libgcc/config/rs6000/float128-sed. It seems like it probably should be a kf symbol, assuming that tf on PPC is IBM double double.

Unless a decision was made to stop adding new symbols for double double at some point? In which case tf for any newer symbols might be IEEE-754 f128.

(Unfortunately I can't test this right now)

@EsmeYi EsmeYi linked a pull request Jul 4, 2024 that will close this issue
@ecnelises
Copy link
Member

I'm afraid that powerpc64le libgcc does not contain symbols: __gnu_h2f_ieee, __extendhftf2, __trunctfhf2? Since ppc64le GCC does not support 16-bit floats

@tgross35
Copy link
Author

tgross35 commented Jul 9, 2024

@ecnelises Does Clang support them? If so, maybe they could be added to compiler-rt.

If you just need something to test against, Rust's compiler_builtins has them already https://github.com/rust-lang/compiler-builtins/blob/06db2de1c098f9c3cf69a52e7a273f19f0dce061/src/float/extend.rs#L101-L120, but with the kf names.

I know that both Clang and GCC talked about defaulting to IEEE binary128 on PPC rather than the IBM 2xdouble. Do you know if this happened yet, or is still happening?

Asking because from section 2.2 here https://gcc.gnu.org/wiki/Ieee128PowerPC, the table mentions that "kf" is always IEEE 754, "tf" is whatever long double is. So it seems like these symbols should probably have kf names, unless long double = binary128 is the default now.

@ecnelises
Copy link
Member

Does Clang support them? If so, maybe they could be added to compiler-rt.

See #89054, LLVM compiler-rt misses a lot of them. I think Rust depends on LLVM's compiler builtins, not (dynamic) libgcc?

Do you know if this happened yet, or is still happening?

Both clang and GCC has not moved yet.

So it seems like these symbols should probably have kf names, unless long double = binary128 is the default now.

It looks like, in libgcc, kf and tf difference is just a matter of build config. Th real issue is that GCC does not support 16-bit float at all on ppc64le, as a result libgcc does not enable any hf related symbols. It's only the case for clang and Linux.

For Rust or FreeBSD, if these symbols are made into in LLVM compiler-rt, we can go and generate references in LLVM backend. (does __gnu_h2f_ieee exist now in compiler-rt?) If we are going to implement LLVM's own, I agree we should use kf instead of tf in name.

@tgross35
Copy link
Author

tgross35 commented Jul 9, 2024

Does Clang support them? If so, maybe they could be added to compiler-rt.

See #89054, LLVM compiler-rt misses a lot of them. I think Rust depends on LLVM's compiler builtins, not (dynamic) libgcc?

Rust does link with the shared library -lgcc_s, and also the port of compiler_builtins. And then compiler_builtins builds c versions from LLVM's compiler-rt for any functions that we don't have ported.

I have tried to be pretty on top of powerpc names so most all the kf symbols on your list are available in Rust's compiler_builtins, so I think it might actually be a step ahead of both LLVM compiler-rt and libgcc (only exceptions: int -> float rust-lang/compiler-builtins#624, div rust-lang/compiler-builtins#622, __cmp[uo]kf2, tf<->kf conversions). Anything marked ppc_alias https://github.com/search?q=repo%3Arust-lang%2Fcompiler-builtins+ppc_alias&type=code.

Do you know if this happened yet, or is still happening?

Both clang and GCC has not moved yet.

Ah that is too bad, thank you for confirming.

So it seems like these symbols should probably have kf names, unless long double = binary128 is the default now.

It looks like, in libgcc, kf and tf difference is just a matter of build config. Th real issue is that GCC does not support 16-bit float at all on ppc64le, as a result libgcc does not enable any hf related symbols. It's only the case for clang and Linux.

For Rust or FreeBSD, if these symbols are made into in LLVM compiler-rt, we can go and generate references in LLVM backend. (does __gnu_h2f_ieee exist now in compiler-rt?) If we are going to implement LLVM's own, I agree we should use kf instead of tf in name.

Thanks for looking into this. __gnu_h2f_ieee exists, but I have no clue what platforms it is built on

COMPILER_RT_ABI float __gnu_h2f_ieee(src_t a) { return __extendhfsf2(a); }
.

To clarify - making LLVM lower correctly shouldn't be blocked on having support in compiler-rt, correct? (e.g. #97677). BE powerpc and powerpc64 lower correctly even though the symbols might not be available, it's just LE that fails on half.

@tgross35
Copy link
Author

tgross35 commented Jul 9, 2024

To clarify - making LLVM lower correctly shouldn't be blocked on having support in compiler-rt, correct? (e.g. #97677). BE powerpc and powerpc64 lower correctly even though the symbols might not be available, it's just LE that fails on half.

Ah, I forgot that part of the problem is that on BE it lowers to tf symbols rather than kf. I'll make a new issue for symbol naming on all ppc platforms, then this issue can stay only about the selection failure on LE (and be fixed by #97677).

Edit: did so in #98126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:PowerPC crash Prefer [crash-on-valid] or [crash-on-invalid] llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants