[PATCH] D55449: [InstCombine] Fix negative GEP offset evaluation for 32-bit pointers

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: spatel, RKSimon.
Herald added a subscriber: llvm-commits.

This fixes https://bugs.llvm.org/show_bug.cgi?id=39908.

The `evaluateGEPOffsetExpression()` function simplifies GEP offsets for use in comparisons against zero, basically by converting X*Scale+Offset==0 to X+Offset/Scale==0 if Scale divides Offset. However, before this is done, Offset is masked down to the pointer size. This results in incorrect results for negative Offsets, because we basically end up dividing the 32-bit offset *zero* extended to 64-bit bits (rather than sign extended).

The masking code could be fixed to properly replicate sign bits. However, as we are operating on inbounds GEPs here, I would expect that we do not have to care about address space overflows, because these would result in poison anyway. If that understanding is correct, then we can just drop this code entirely, which is what this patch does.


Repository:
  rL LLVM

https://reviews.llvm.org/D55449

Files:
  lib/Transforms/InstCombine/InstCombineCompares.cpp
  test/Transforms/InstCombine/pr39908.ll


Index: test/Transforms/InstCombine/pr39908.ll
===================================================================
--- /dev/null
+++ test/Transforms/InstCombine/pr39908.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+target datalayout = "p:32:32"
+
+%S = type { [2 x i32] }
+
+define i1 @foo([0 x %S]* %p, i32 %n) {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[N:%.*]], 1
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %start.cast = bitcast [0 x %S]* %p to %S*
+  %end = getelementptr inbounds [0 x %S], [0 x %S]* %p, i32 0, i32 %n, i32 0, i32 0
+  %end.cast = bitcast i32* %end to %S*
+  %last = getelementptr inbounds %S, %S* %end.cast, i32 -1
+  %cmp = icmp eq %S* %last, %start.cast
+  ret i1 %cmp
+}
Index: lib/Transforms/InstCombine/InstCombineCompares.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -521,13 +521,6 @@
     return VariableIdx;
   }
 
-  // Otherwise, there is an index.  The computation we will do will be modulo
-  // the pointer size, so get it.
-  uint64_t PtrSizeMask = ~0ULL >> (64-IntPtrWidth);
-
-  Offset &= PtrSizeMask;
-  VariableScale &= PtrSizeMask;
-
   // To do this transformation, any constant index must be a multiple of the
   // variable scale factor.  For example, we can evaluate "12 + 4*i" as "3 + i",
   // but we can't evaluate "10 + 3*i" in terms of i.  Check that the offset is a


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55449.177257.patch
Type: text/x-patch
Size: 1578 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181207/3fff621a/attachment.bin>