search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.04k stars 11.98k forks source link

[RISCV] `fpext` crashes on `half` type #63816

Closed HazyFish closed 1 year ago

HazyFish commented 1 year ago

Description

When d (Standard Extension for Double-Precision Floating-Point) feature is on, the following code crashes RISCV backend with assertion error VT.isInteger() == MemVT.isInteger() && "Cannot convert from FP to Int or Int -> FP!".

Cause

https://github.com/llvm/llvm-project/blob/b10899d869954e1426684cbc20a43d7303075d49/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp#L8210-L8229

The param VT is i64 type and MemVT is f16, which caused the assertion to fail.

Minimal Reproduction

https://godbolt.org/z/EK698bYWv

Code

define void @test(ptr %0, ptr %1) {
  %V1 = load <8 x half>, ptr %0
  %V2 = fpext <8 x half> %V1 to <8 x double>
  store <8 x double> %V2, ptr %1
  ret void
}

Stack Trace

llc: /root/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:8222: llvm::SDValue llvm::SelectionDAG::getLoad(llvm::ISD::MemIndexedMode, llvm::ISD::LoadExtType, llvm::EVT, const llvm::SDLoc&, llvm::SDValue, llvm::SDValue, llvm::SDValue, llvm::EVT, llvm::MachineMemOperand*): Assertion `VT.isInteger() == MemVT.isInteger() && "Cannot convert from FP to Int or Int -> FP!"' failed.
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-assertions-trunk/bin/llc -o /app/output.s -x86-asm-syntax=intel -mtriple=riscv64 -mattr=+d <source>
1.  Running pass 'Function Pass Manager' on module '<source>'.
2.  Running pass 'RISC-V DAG->DAG Pattern Instruction Selection' on function '@test'
 #0 0x0000557e3ad2b15f llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x354b15f)
 #1 0x0000557e3ad288b4 SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f11c56f2420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #3 0x00007f11c51bf00b raise (/lib/x86_64-linux-gnu/libc.so.6+0x4300b)
 #4 0x00007f11c519e859 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22859)
 #5 0x00007f11c519e729 (/lib/x86_64-linux-gnu/libc.so.6+0x22729)
 #6 0x00007f11c51affd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
 #7 0x0000557e3aa9739a (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x32b739a)
 #8 0x0000557e3aa9e6ed llvm::SelectionDAG::getExtLoad(llvm::ISD::LoadExtType, llvm::SDLoc const&, llvm::EVT, llvm::SDValue, llvm::SDValue, llvm::EVT, llvm::MachineMemOperand*) (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x32be6ed)
 #9 0x0000557e3a9c31b8 (anonymous namespace)::SelectionDAGLegalize::LegalizeLoadOps(llvm::SDNode*) LegalizeDAG.cpp:0:0
#10 0x0000557e3a9e2765 (anonymous namespace)::SelectionDAGLegalize::LegalizeOp(llvm::SDNode*) (.part.0) LegalizeDAG.cpp:0:0
#11 0x0000557e3a9e630c llvm::SelectionDAG::Legalize() (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x320630c)
#12 0x0000557e3aaf1cec llvm::SelectionDAGISel::CodeGenAndEmitDAG() (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x3311cec)
#13 0x0000557e3aaf54c8 llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x33154c8)
#14 0x0000557e3aaf7192 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (.part.0) SelectionDAGISel.cpp:0:0
#15 0x0000557e39fca6ce llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (.part.0) MachineFunctionPass.cpp:0:0
#16 0x0000557e3a5647f1 llvm::FPPassManager::runOnFunction(llvm::Function&) (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x2d847f1)
#17 0x0000557e3a564a39 llvm::FPPassManager::runOnModule(llvm::Module&) (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x2d84a39)
#18 0x0000557e3a5652b2 llvm::legacy::PassManagerImpl::run(llvm::Module&) (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x2d852b2)
#19 0x0000557e38077634 compileModule(char**, llvm::LLVMContext&) llc.cpp:0:0
#20 0x0000557e37fbff06 main (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x7dff06)
#21 0x00007f11c51a0083 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24083)
#22 0x0000557e3806da1e _start (/opt/compiler-explorer/clang-assertions-trunk/bin/llc+0x88da1e)
Program terminated with signal: SIGSEGV
Compiler returned: 139
HazyFish commented 1 year ago

@DataCorrupted

llvmbot commented 1 year ago

@llvm/issue-subscribers-backend-risc-v

wangpc-pp commented 1 year ago

Somehow extload from f16 to i64 is legal.

topperc commented 1 year ago

Somehow extload from f16 to i64 is legal.

The table for legality defaults to all 0s which means Legal. Nothing makes it not legal. I'm working on a patch.

wangpc-pp commented 1 year ago

OK. FYI, I just added two lines to fix this (extload from f16 to i32 has the same problem):

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 2ef55d6d3b42..0a9012df5933 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -425,6 +425,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
     setOperationAction(ISD::BR_CC, MVT::f32, Expand);
     setOperationAction(FPOpToExpand, MVT::f32, Expand);
     setLoadExtAction(ISD::EXTLOAD, MVT::f32, MVT::f16, Expand);
+    setLoadExtAction(ISD::EXTLOAD, MVT::i32, MVT::f16, Expand);
     setTruncStoreAction(MVT::f32, MVT::f16, Expand);
     setOperationAction(ISD::IS_FPCLASS, MVT::f32, Custom);
     setOperationAction(ISD::BF16_TO_FP, MVT::f32, Custom);
@@ -464,6 +465,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
     setTruncStoreAction(MVT::f64, MVT::f32, Expand);
     setOperationAction(FPOpToExpand, MVT::f64, Expand);
     setLoadExtAction(ISD::EXTLOAD, MVT::f64, MVT::f16, Expand);
+    setLoadExtAction(ISD::EXTLOAD, MVT::i64, MVT::f16, Expand);
     setTruncStoreAction(MVT::f64, MVT::f16, Expand);
     setOperationAction(ISD::IS_FPCLASS, MVT::f64, Custom);
     setOperationAction(ISD::BF16_TO_FP, MVT::f64, Custom);
topperc commented 1 year ago

OK. FYI, I just added two lines to fix this (extload from f16 to i32 has the same problem):

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 2ef55d6d3b42..0a9012df5933 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -425,6 +425,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
     setOperationAction(ISD::BR_CC, MVT::f32, Expand);
     setOperationAction(FPOpToExpand, MVT::f32, Expand);
     setLoadExtAction(ISD::EXTLOAD, MVT::f32, MVT::f16, Expand);
+    setLoadExtAction(ISD::EXTLOAD, MVT::i32, MVT::f16, Expand);
     setTruncStoreAction(MVT::f32, MVT::f16, Expand);
     setOperationAction(ISD::IS_FPCLASS, MVT::f32, Custom);
     setOperationAction(ISD::BF16_TO_FP, MVT::f32, Custom);
@@ -464,6 +465,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
     setTruncStoreAction(MVT::f64, MVT::f32, Expand);
     setOperationAction(FPOpToExpand, MVT::f64, Expand);
     setLoadExtAction(ISD::EXTLOAD, MVT::f64, MVT::f16, Expand);
+    setLoadExtAction(ISD::EXTLOAD, MVT::i64, MVT::f16, Expand);
     setTruncStoreAction(MVT::f64, MVT::f16, Expand);
     setOperationAction(ISD::IS_FPCLASS, MVT::f64, Custom);
     setOperationAction(ISD::BF16_TO_FP, MVT::f64, Custom);

I'm going to fix LegalizeDAG to not mix int and FP types when trying to legalize extending loads. Targets shouldn't have to mark non-sensical type actions to prevent this.