[PATCH] D90610: [Inline] Fix in handling of ptrtoint in InlineCost
Mikael Holmén via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 3 00:04:12 PST 2020
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>
More information about the llvm-commits
mailing list