[PATCH] D33338: [InstCombineCasts] Take in account final size when transforming sext->lshr->trunc patterns

[Davide Italiano via Phabricator via llvm-commits]

davide created this revision.

Counter example where this miscompiles:

define i8 @tinky() {

  %sext = sext i1 1 to i16
  %hibit = lshr i16 %sext, 15
  %tr = trunc i16 %hibit to i8
  ret i8 %tr

}

define i8 @winky() {

  %sext = sext i1 1 to i8
  ret i8 %sext

}

which gets folded to:

define i8 @tinky() {

  ret i8 1

}

define i8 @winky() {

  ret i8 -1

}

Alive confirms: http://rise4fun.com/Alive/plX


https://reviews.llvm.org/D33338

Files:
  lib/Transforms/InstCombine/InstCombineCasts.cpp
  test/Transforms/InstCombine/cast.ll


Index: test/Transforms/InstCombine/cast.ll
===================================================================
--- test/Transforms/InstCombine/cast.ll
+++ test/Transforms/InstCombine/cast.ll
@@ -1470,3 +1470,16 @@
   %D = trunc i96 %C to i32
   ret i32 %D
 }
+; Don't turn this in an `ashr`.
+; CHECK-LABEL: @PR33078(
+; CHECK-NEXT:    [[B:%.*]] = sext i3 %x to i16
+; CHECK-NEXT:    [[C:%.*]] = lshr i16 [[B]], 13
+; CHECK-NEXT:    [[D:%.*]] = trunc i16 [[C]] to i8
+; CHECK-NEXT:    ret i8 [[D]]
+
+define i8 @PR33078(i3 %x) {
+  %B = sext i3 %x to i16
+  %C = lshr i16 %B, 13
+  %D = trunc i16 %C to i8
+  ret i8 %D
+}
Index: lib/Transforms/InstCombine/InstCombineCasts.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -595,19 +595,25 @@
     Value *SExt = cast<Instruction>(Src)->getOperand(0);
     const unsigned SExtSize = SExt->getType()->getPrimitiveSizeInBits();
     const unsigned ASize = A->getType()->getPrimitiveSizeInBits();
+    const unsigned CISize = CI.getType()->getPrimitiveSizeInBits();
     unsigned ShiftAmt = Cst->getZExtValue();
+
     // This optimization can be only performed when zero bits generated by
     // the original lshr aren't pulled into the value after truncation, so we
     // can only shift by values no larger than the number of extension bits.
     // FIXME: Instead of bailing when the shift is too large, use and to clear
     // the extra bits.
-    if (SExt->hasOneUse() && ShiftAmt <= SExtSize - ASize) {
-      // If shifting by the size of the original value in bits or more, it is
-      // being filled with the sign bit, so shift by ASize-1 to avoid ub.
-      Value *Shift = Builder->CreateAShr(A, std::min(ShiftAmt, ASize-1));
-      Shift->takeName(Src);
-      return CastInst::CreateIntegerCast(Shift, CI.getType(), true);
-    }
+    const unsigned MaxAmt = SExtSize - CISize;
+    if (CISize == ASize)
+      if (ShiftAmt <= MaxAmt)
+        return BinaryOperator::CreateAShr(A, ConstantInt::get(CI.getType(),
+                                                     std::min(ShiftAmt, ASize - 1)));
+    if (CISize > ASize)
+      if (SExt->hasOneUse() && ShiftAmt <= MaxAmt) {
+        Value *Shift = Builder->CreateAShr(A, std::min(ShiftAmt, ASize-1));
+        Shift->takeName(Src);
+        return CastInst::CreateIntegerCast(Shift, CI.getType(), true);
+      }
   }
 
   if (Instruction *I = shrinkBitwiseLogic(CI))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33338.99493.patch
Type: text/x-patch
Size: 2514 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170518/8b34a51e/attachment.bin>