Skip to content

Use getSignedTargetConstant for offset #141149

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

Merged
merged 2 commits into from
May 26, 2025

Conversation

scui-ibm
Copy link
Contributor

This is to fix an assertion failure with PeepholePPC64. The load/store offset can be negative. A reduced case from one of our failures is added as well.

@scui-ibm scui-ibm self-assigned this May 22, 2025
@llvmbot
Copy link
Member

llvmbot commented May 22, 2025

@llvm/pr-subscribers-backend-powerpc

Author: Shimin Cui (scui-ibm)

Changes

This is to fix an assertion failure with PeepholePPC64. The load/store offset can be negative. A reduced case from one of our failures is added as well.


Full diff: https://github.com/llvm/llvm-project/pull/141149.diff

2 Files Affected:

  • (modified) llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp (+2-2)
  • (added) llvm/test/CodeGen/PowerPC/signed-offset.ll (+33)
diff --git a/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp b/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
index 7147506bbd89b..f921032356d65 100644
--- a/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
@@ -7858,8 +7858,8 @@ void PPCDAGToDAGISel::PeepholePPC64() {
         if (!isInt<16>(Offset))
           continue;
 
-        ImmOpnd = CurDAG->getTargetConstant(Offset, SDLoc(ImmOpnd),
-                                            ImmOpnd.getValueType());
+        ImmOpnd = CurDAG->getSignedTargetConstant(Offset, SDLoc(ImmOpnd),
+                                                  ImmOpnd.getValueType());
       } else if (Offset != 0) {
         // This optimization is performed for non-TOC-based local-[exec|dynamic]
         // accesses.
diff --git a/llvm/test/CodeGen/PowerPC/signed-offset.ll b/llvm/test/CodeGen/PowerPC/signed-offset.ll
new file mode 100644
index 0000000000000..d12f99ead644b
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/signed-offset.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -verify-machineinstrs < %s -mcpu=ppc -mtriple powerpc-ibm-aix-xcoff | FileCheck %s
+
+%_type = type <{ [90112 x i8] }>
+
+@g = external global %_type
+
+define void @foo(ptr %p) {
+; CHECK-LABEL: foo:
+; CHECK:     	 stfd 31, 0(31)
+; CHECK-NEXT:    lwz 3, -32(30)
+; CHECK-NEXT:    lwz 4, -28(30)
+; CHECK-NEXT:    cmplwi 3, 0
+; CHECK-NEXT:    cmpwi 1, 3, 0
+; CHECK-NEXT:    crandc 20, 5, 2
+; CHECK-NEXT:    cmpwi 1, 4, 0
+entry:
+  %0 = load double, ptr getelementptr inbounds nuw (i8, ptr @g, i32 83272), align 8
+  %1 = call i32 @bar(ptr getelementptr inbounds nuw (i8, ptr @g, i32 83272))
+  %2 = call i32 @bar(ptr getelementptr inbounds nuw (i8, ptr @g, i32 83240))
+  store double %0, ptr %p, align 8
+  %3 = load i64, ptr getelementptr inbounds nuw (i8, ptr @g, i32 83240), align 8
+  %4 = icmp slt i64 %3, 1
+  br i1 %4, label %then, label %else
+
+then:                                                     ; preds = %entry
+  ret void
+
+else:                                                     ; preds = %entry
+  ret void
+}
+
+declare signext i32 @bar(ptr)

@scui-ibm scui-ibm requested review from amy-kwan, lei137 and topperc May 23, 2025 13:57
@topperc topperc requested a review from nikic May 23, 2025 16:13
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@amy-kwan amy-kwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this LGTM.

@scui-ibm scui-ibm merged commit b1017a4 into llvm:main May 26, 2025
11 checks passed
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
This is to fix an assertion failure with PeepholePPC64. The load/store
offset can be negative. A reduced case from one of our failures is added
as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants