[PATCH] D90610: [Inline] Fix in handling of ptrtoint in InlineCost

[Mikael Holmén via Phabricator via llvm-commits]

uabelho updated this revision to Diff 302480.
uabelho added a comment.

Changed names of the testcase functions and changed to zext in visitPtrToInt.


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

https://reviews.llvm.org/D90610

Files:
  llvm/lib/Analysis/InlineCost.cpp
  llvm/test/Transforms/Inline/inline-ptrtoint-different-sizes.ll


Index: llvm/test/Transforms/Inline/inline-ptrtoint-different-sizes.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-ptrtoint-different-sizes.ll
@@ -0,0 +1,40 @@
+; RUN: opt < %s -inline -S | FileCheck %s
+
+; InlineCost used to have problems with the ptrtoint, leading to
+; crashes when visiting the trunc in pr48908_help and the icmp in
+; pr38500_help.
+
+target datalayout = "p:16:16"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @pr48980_help(i16* %p) {
+  %cast = ptrtoint i16* %p to i32
+  %sub = sub i32 %cast, %cast
+  %conv = trunc i32 %sub to i16
+  ret void
+}
+
+define void @pr48980(i16* %x) {
+  call void @pr48980_help(i16* %x)
+  ret void
+}
+
+; CHECK-LABEL: @pr48980(i16* %x)
+; CHECK-NOT:     call
+; CHECK:         ret void
+
+define void @pr38500_help(i16* %p) {
+  %cast = ptrtoint i16* %p to i32
+  %sub = sub i32 %cast, %cast
+  %cmp = icmp eq i32 %sub, 0
+  ret void
+}
+
+define void @pr38500(i16* %x) {
+  call void @pr38500_help(i16* %x)
+  ret void
+}
+
+; CHECK-LABEL: @pr38500(i16* %x)
+; CHECK-NOT:     call
+; CHECK:         ret void
Index: llvm/lib/Analysis/InlineCost.cpp
===================================================================
--- llvm/lib/Analysis/InlineCost.cpp
+++ llvm/lib/Analysis/InlineCost.cpp
@@ -1104,8 +1104,29 @@
   if (IntegerSize >= DL.getPointerSizeInBits(AS)) {
     std::pair<Value *, APInt> BaseAndOffset =
         ConstantOffsetPtrs.lookup(I.getOperand(0));
-    if (BaseAndOffset.first)
-      ConstantOffsetPtrs[&I] = BaseAndOffset;
+    if (BaseAndOffset.first) {
+      // If the sizes of the result type and the stored BaseAndOffset match
+      // exactly we can reuse that as is. Otherwise we need to create a new
+      // offset of the correct size and use that instead.
+      // Using an offset of the wrong size may lead to problems later on if we
+      // e.g. have
+      //   %i = ptrtoint i16* %p to i32
+      //   %s = sub i32 %i, %i
+      //   %t = trunc i32 %s to i16
+      // with the pointer size being 16, we might have stored BaseAndOffset
+      // for %p with offset size 16 bits. If we just reuse that 16 bit
+      // offset also for %i, %s will end up as i16 0 and then an assertion
+      // triggers when handling the trunc since a trunc from i16 to i16 isn't
+      // valid.
+      if (IntegerSize == BaseAndOffset.second.getBitWidth())
+        ConstantOffsetPtrs[&I] = BaseAndOffset;
+      else {
+        APInt OffsetWithCorrectSize = BaseAndOffset.second.zext(IntegerSize);
+        std::pair<Value *, APInt> BaseAndOffsetWithCorrectSize =
+          {BaseAndOffset.first, OffsetWithCorrectSize};
+        ConstantOffsetPtrs[&I] = BaseAndOffsetWithCorrectSize;
+      }
+    }
   }
 
   // This is really weird. Technically, ptrtoint will disable SROA. However,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90610.302480.patch
Type: text/x-patch
Size: 2860 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201103/0d8cad21/attachment.bin>