[PowerPC] Drop invalid range metadata when lowering i64 load to fp in INT_TO_FP#198705
Conversation
|
@llvm/pr-subscribers-backend-powerpc Author: Amy Kwan (amy-kwan) ChangesWhen lowering an i64 load in LowerINT_TO_FP, we were forwarding the original !range metadata to a new f64 load. This is invalid because the metadata no longer matches the value type/semantics, and can trigger assertions when lowering i64 to fp (double or float) conversions. This patch fixes this by passing a nullptr for the Ranges operand when calling getLoad() and adds extra test cases to cover signed/unsigned i64 to f32/f64 conversions and to ensure they do not assert when the !range metadata is present. The assertion this patch attempts to fix is: This assert was originally seen when building Rust on AIX. Full diff: https://github.com/llvm/llvm-project/pull/198705.diff 2 Files Affected:
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index d8ed51ec9b26f..d121e8e2cc578 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -8865,7 +8865,7 @@ SDValue PPCTargetLowering::LowerINT_TO_FP(SDValue Op,
MachineFunction &MF = DAG.getMachineFunction();
if (canReuseLoadAddress(SINT, MVT::i64, RLI, DAG)) {
Bits = DAG.getLoad(MVT::f64, dl, RLI.Chain, RLI.Ptr, RLI.MPI,
- RLI.Alignment, RLI.MMOFlags(), RLI.AAInfo, RLI.Ranges);
+ RLI.Alignment, RLI.MMOFlags(), RLI.AAInfo, nullptr);
if (RLI.ResChain)
DAG.makeEquivalentMemoryOrdering(RLI.ResChain, Bits.getValue(1));
} else if (Subtarget.hasLFIWAX() &&
diff --git a/llvm/test/CodeGen/PowerPC/ppc-i64-to-fp.ll b/llvm/test/CodeGen/PowerPC/ppc-i64-to-fp.ll
new file mode 100644
index 0000000000000..420163f6ad101
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/ppc-i64-to-fp.ll
@@ -0,0 +1,124 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mcpu=pwr8 -ppc-asm-full-reg-names -mtriple=powerpc64-ibm-aix < %s | \
+; RUN: FileCheck %s --check-prefix=CHECK-AIX64
+; RUN: llc -mcpu=pwr8 -ppc-asm-full-reg-names -mtriple=powerpc-ibm-aix < %s | \
+; RUN: FileCheck %s --check-prefix=CHECK-AIX32
+; RUN: llc -mcpu=pwr8 -ppc-asm-full-reg-names -mtriple=powerpc64le-unknown-linux-gnu < %s | \
+; RUN: FileCheck %s --check-prefix=CHECK-LINUX
+
+; Test that assertions related to the range metadata when converting from i64
+; to i32 does not occur.
+
+define float @case_i64_uitofp_f32(ptr %p) {
+; CHECK-AIX64-LABEL: case_i64_uitofp_f32:
+; CHECK-AIX64: # %bb.0: # %entry
+; CHECK-AIX64-NEXT: lfd f0, 0(r3)
+; CHECK-AIX64-NEXT: xscvuxdsp f1, f0
+; CHECK-AIX64-NEXT: blr
+;
+; CHECK-AIX32-LABEL: case_i64_uitofp_f32:
+; CHECK-AIX32: # %bb.0: # %entry
+; CHECK-AIX32-NEXT: lwz r4, 4(r3)
+; CHECK-AIX32-NEXT: stw r4, -4(r1)
+; CHECK-AIX32-NEXT: lwz r3, 0(r3)
+; CHECK-AIX32-NEXT: stw r3, -8(r1)
+; CHECK-AIX32-NEXT: lfd f0, -8(r1)
+; CHECK-AIX32-NEXT: xscvuxdsp f1, f0
+; CHECK-AIX32-NEXT: blr
+;
+; CHECK-LINUX-LABEL: case_i64_uitofp_f32:
+; CHECK-LINUX: # %bb.0: # %entry
+; CHECK-LINUX-NEXT: lfd f0, 0(r3)
+; CHECK-LINUX-NEXT: xscvuxdsp f1, f0
+; CHECK-LINUX-NEXT: blr
+entry:
+ %x = load i64, ptr %p, align 8, !range !0
+ %y = uitofp i64 %x to float
+ ret float %y
+}
+
+define double @case_i64_sitofp_f64(ptr %p) {
+; CHECK-AIX64-LABEL: case_i64_sitofp_f64:
+; CHECK-AIX64: # %bb.0: # %entry
+; CHECK-AIX64-NEXT: lfd f0, 0(r3)
+; CHECK-AIX64-NEXT: xscvsxddp f1, f0
+; CHECK-AIX64-NEXT: blr
+;
+; CHECK-AIX32-LABEL: case_i64_sitofp_f64:
+; CHECK-AIX32: # %bb.0: # %entry
+; CHECK-AIX32-NEXT: lwz r4, 4(r3)
+; CHECK-AIX32-NEXT: stw r4, -4(r1)
+; CHECK-AIX32-NEXT: lwz r3, 0(r3)
+; CHECK-AIX32-NEXT: stw r3, -8(r1)
+; CHECK-AIX32-NEXT: lfd f0, -8(r1)
+; CHECK-AIX32-NEXT: xscvsxddp f1, f0
+; CHECK-AIX32-NEXT: blr
+;
+; CHECK-LINUX-LABEL: case_i64_sitofp_f64:
+; CHECK-LINUX: # %bb.0: # %entry
+; CHECK-LINUX-NEXT: lfd f0, 0(r3)
+; CHECK-LINUX-NEXT: xscvsxddp f1, f0
+; CHECK-LINUX-NEXT: blr
+entry:
+ %x = load i64, ptr %p, align 8, !range !0
+ %y = sitofp i64 %x to double
+ ret double %y
+}
+
+define double @case_i64_uitofp_f64(ptr %p) {
+; CHECK-AIX64-LABEL: case_i64_uitofp_f64:
+; CHECK-AIX64: # %bb.0: # %entry
+; CHECK-AIX64-NEXT: lfd f0, 0(r3)
+; CHECK-AIX64-NEXT: xscvuxddp f1, f0
+; CHECK-AIX64-NEXT: blr
+;
+; CHECK-AIX32-LABEL: case_i64_uitofp_f64:
+; CHECK-AIX32: # %bb.0: # %entry
+; CHECK-AIX32-NEXT: lwz r4, 4(r3)
+; CHECK-AIX32-NEXT: stw r4, -4(r1)
+; CHECK-AIX32-NEXT: lwz r3, 0(r3)
+; CHECK-AIX32-NEXT: stw r3, -8(r1)
+; CHECK-AIX32-NEXT: lfd f0, -8(r1)
+; CHECK-AIX32-NEXT: xscvuxddp f1, f0
+; CHECK-AIX32-NEXT: blr
+;
+; CHECK-LINUX-LABEL: case_i64_uitofp_f64:
+; CHECK-LINUX: # %bb.0: # %entry
+; CHECK-LINUX-NEXT: lfd f0, 0(r3)
+; CHECK-LINUX-NEXT: xscvuxddp f1, f0
+; CHECK-LINUX-NEXT: blr
+entry:
+ %x = load i64, ptr %p, align 8, !range !0
+ %y = uitofp i64 %x to double
+ ret double %y
+}
+
+define float @case_i64_sitofp_f32(ptr %p) {
+; CHECK-AIX64-LABEL: case_i64_sitofp_f32:
+; CHECK-AIX64: # %bb.0: # %entry
+; CHECK-AIX64-NEXT: lfd f0, 0(r3)
+; CHECK-AIX64-NEXT: xscvsxdsp f1, f0
+; CHECK-AIX64-NEXT: blr
+;
+; CHECK-AIX32-LABEL: case_i64_sitofp_f32:
+; CHECK-AIX32: # %bb.0: # %entry
+; CHECK-AIX32-NEXT: lwz r4, 4(r3)
+; CHECK-AIX32-NEXT: stw r4, -4(r1)
+; CHECK-AIX32-NEXT: lwz r3, 0(r3)
+; CHECK-AIX32-NEXT: stw r3, -8(r1)
+; CHECK-AIX32-NEXT: lfd f0, -8(r1)
+; CHECK-AIX32-NEXT: xscvsxdsp f1, f0
+; CHECK-AIX32-NEXT: blr
+;
+; CHECK-LINUX-LABEL: case_i64_sitofp_f32:
+; CHECK-LINUX: # %bb.0: # %entry
+; CHECK-LINUX-NEXT: lfd f0, 0(r3)
+; CHECK-LINUX-NEXT: xscvsxdsp f1, f0
+; CHECK-LINUX-NEXT: blr
+entry:
+ %x = load i64, ptr %p, align 8, !range !0
+ %y = sitofp i64 %x to float
+ ret float %y
+}
+
+!0 = !{i64 1, i64 0}
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
| ; RUN: llc -mcpu=pwr8 -ppc-asm-full-reg-names -mtriple=powerpc-ibm-aix < %s | \ | ||
| ; RUN: FileCheck %s --check-prefix=CHECK-AIX32 | ||
| ; RUN: llc -mcpu=pwr8 -ppc-asm-full-reg-names -mtriple=powerpc64le-unknown-linux-gnu < %s | \ | ||
| ; RUN: FileCheck %s --check-prefix=CHECK-LINUX |
There was a problem hiding this comment.
nit: maybe we can just use default CHECK for 64bit runs since I don't see a diff in the asm generated.
513a0a9 to
d4e6eb7
Compare
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
… INT_TO_FP When lowering an i64 load in LowerINT_TO_FP, we were forwarding the original !range metadata to a new f64 load. This is invalid because the metadata no longer matches the value type/semantics, and can trigger assertions when lowering i64 to fp (double or float) conversions. This patch fixes this by passing a nullptr for the Ranges operand when calling getLoad() and adds extra test cases to cover signed/unsigned i64 to f32/f64 conversions and to ensure they do not assert when the !range metadata is present. The assertion this patch attempts to fix is: ``` Assertion failed: (!MMO->getRanges() || (mdconst::extract<ConstantInt>(MMO->getRanges()->getOperand(0)) ->getBitWidth() == MemVT.getScalarSizeInBits() && MemVT.isInteger())) && "Range metadata and load type must match!" ``` This assert was originally seen when building Rust on AIX.
d4e6eb7 to
a397685
Compare
|
/cherry-pick 78f5f77 |
|
/pull-request #199985 |
… INT_TO_FP (llvm#198705) When lowering an i64 load in LowerINT_TO_FP, we were forwarding the original !range metadata to a new f64 load. This is invalid because the metadata no longer matches the value type/semantics, and can trigger assertions when lowering i64 to fp (double or float) conversions. This patch fixes this by passing a nullptr for the Ranges operand when calling getLoad() and adds extra test cases to cover signed/unsigned i64 to f32/f64 conversions and to ensure they do not assert when the !range metadata is present. The assertion this patch attempts to fix is: ``` Assertion failed: (!MMO->getRanges() || (mdconst::extract<ConstantInt>(MMO->getRanges()->getOperand(0)) ->getBitWidth() == MemVT.getScalarSizeInBits() && MemVT.isInteger())) && "Range metadata and load type must match!" ``` This assert was originally seen when building Rust on AIX. (cherry picked from commit 78f5f77)
… INT_TO_FP (llvm#198705) When lowering an i64 load in LowerINT_TO_FP, we were forwarding the original !range metadata to a new f64 load. This is invalid because the metadata no longer matches the value type/semantics, and can trigger assertions when lowering i64 to fp (double or float) conversions. This patch fixes this by passing a nullptr for the Ranges operand when calling getLoad() and adds extra test cases to cover signed/unsigned i64 to f32/f64 conversions and to ensure they do not assert when the !range metadata is present. The assertion this patch attempts to fix is: ``` Assertion failed: (!MMO->getRanges() || (mdconst::extract<ConstantInt>(MMO->getRanges()->getOperand(0)) ->getBitWidth() == MemVT.getScalarSizeInBits() && MemVT.isInteger())) && "Range metadata and load type must match!" ``` This assert was originally seen when building Rust on AIX. (cherry picked from commit 78f5f77)
When lowering an i64 load in LowerINT_TO_FP, we were forwarding the original !range metadata to a new f64 load. This is invalid because the metadata no longer matches the value type/semantics, and can trigger assertions when lowering i64 to fp (double or float) conversions.
This patch fixes this by passing a nullptr for the Ranges operand when calling getLoad() and adds extra test cases to cover signed/unsigned i64 to f32/f64 conversions and to ensure they do not assert when the !range metadata is present.
The assertion this patch attempts to fix is:
This assert was originally seen when building Rust on AIX.