[PATCH] D149507: [SeparateConstOffsetFromGEP] Fix bug handling negative offsets

[Tom Stellard via Phabricator via llvm-commits]

tstellar updated this revision to Diff 518708.
tstellar added a comment.

Delete now dead code that was negating the constant.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149507/new/

https://reviews.llvm.org/D149507

Files:
  llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
  llvm/test/Transforms/SeparateConstOffsetFromGEP/pr62379-zeroext-negative.ll


Index: llvm/test/Transforms/SeparateConstOffsetFromGEP/pr62379-zeroext-negative.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/SeparateConstOffsetFromGEP/pr62379-zeroext-negative.ll
@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+; RUN: opt -S '-passes=separate-const-offset-from-gep<lower-gep>' < %s | FileCheck %s
+
+; These tests check that this pass does not incorrectly handle negative offsets.
+
+ at c = internal constant [4 x i32] [i32 0, i32 1, i32 2, i32 3]
+
+; FIXME: We could optimize this case, but we don't currently.  This test just
+; checks that we don't miscompile it.
+define i32 @sub_positive(i32 %a, i32 %b, ptr %ptr) {
+; CHECK-LABEL: define i32 @sub_positive
+; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]], ptr [[PTR:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = sub nuw nsw i32 15, [[A]]
+; CHECK-NEXT:    [[TMP1:%.*]] = sub nuw nsw i32 [[B]], [[TMP0]]
+; CHECK-NEXT:    [[TMP2:%.*]] = zext i32 [[TMP1]] to i64
+; CHECK-NEXT:    [[TMP3:%.*]] = getelementptr inbounds [4 x i32], ptr [[PTR]], i64 0, i64 [[TMP2]]
+; CHECK-NEXT:    [[TMP4:%.*]] = load i32, ptr [[TMP3]], align 4
+; CHECK-NEXT:    ret i32 [[TMP4]]
+;
+entry:
+  %0 = sub nuw nsw i32 15, %a
+  %1 = sub nuw nsw i32 %b, %0
+  %2 = zext i32 %1 to i64
+  %3 = getelementptr inbounds [4 x i32], ptr %ptr, i64 0, i64 %2
+  %4 = load i32, ptr %3
+  ret i32 %4
+}
Index: llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -521,6 +521,12 @@
       !haveNoCommonBitsSet(LHS, RHS, DL, nullptr, BO, DT))
     return false;
 
+  // FIXME: We don't currently support constants from the RHS of subs,
+  // when we are zero-extended, because we need a way to zero-extended
+  // them before they are negated.
+  if (ZeroExtended && !SignExtended && BO->getOpcode() == Instruction::Sub)
+    return false;
+
   // In addition, tracing into BO requires that its surrounding s/zext (if
   // any) is distributable to both operands.
   //
@@ -587,10 +593,6 @@
 
   ConstantOffset = find(BO->getOperand(1), SignExtended, ZeroExtended,
                         /* NonNegative */ false);
-  // If U is a sub operator, negate the constant offset found in the right
-  // operand.
-  if (BO->getOpcode() == Instruction::Sub)
-    ConstantOffset = -ConstantOffset;
 
   // If RHS wasn't a suitable candidate either, reset the chain again.
   if (ConstantOffset == 0)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149507.518708.patch
Type: text/x-patch
Size: 2675 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230502/e4e0d05a/attachment.bin>