From: Davide Italiano Date: Sun, 21 May 2017 20:30:27 +0000 (+0000) Subject: [InstCombine] Take in account the size in sext->lshr->trunc patterns. X-Git-Tag: android-x86-7.1-r4~16000 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=4276950084b80cbae3d716fb691cfe9700056786;p=android-x86%2Fexternal-llvm.git [InstCombine] Take in account the size in sext->lshr->trunc patterns. Otherwise we end up miscompiling, transforming: define i8 @tinky() { %sext = sext i1 1 to i16 %hibit = lshr i16 %sext, 15 %tr = trunc i16 %hibit to i8 ret i8 %tr } into: %sext = sext i1 1 to i8 ret i8 %sext and the first get folded to ret i8 1, while the second gets folded to ret i8 -1. Eventually we should get rid of this transform entirely, but for now, this at least fixes a know correctness bug. Differential Revision: https://reviews.llvm.org/D33338 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@303513 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Transforms/InstCombine/InstCombineCasts.cpp b/lib/Transforms/InstCombine/InstCombineCasts.cpp index 001a4bcf16f..f4bf5221f6a 100644 --- a/lib/Transforms/InstCombine/InstCombineCasts.cpp +++ b/lib/Transforms/InstCombine/InstCombineCasts.cpp @@ -585,6 +585,7 @@ Instruction *InstCombiner::visitTrunc(TruncInst &CI) { return CastInst::CreateIntegerCast(Shift, DestTy, false); } + // FIXME: We should canonicalize to zext/trunc and remove this transform. // Transform trunc(lshr (sext A), Cst) to ashr A, Cst to eliminate type // conversion. // It works because bits coming from sign extension have the same value as @@ -595,18 +596,24 @@ Instruction *InstCombiner::visitTrunc(TruncInst &CI) { Value *SExt = cast(Src)->getOperand(0); const unsigned SExtSize = SExt->getType()->getPrimitiveSizeInBits(); const unsigned ASize = A->getType()->getPrimitiveSizeInBits(); + const unsigned CISize = CI.getType()->getPrimitiveSizeInBits(); + const unsigned MaxAmt = SExtSize - std::max(CISize, ASize); 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); + if (ShiftAmt <= MaxAmt) { + if (CISize == ASize) + return BinaryOperator::CreateAShr(A, ConstantInt::get(CI.getType(), + std::min(ShiftAmt, ASize - 1))); + if (SExt->hasOneUse()) { + Value *Shift = Builder->CreateAShr(A, std::min(ShiftAmt, ASize-1)); + Shift->takeName(Src); + return CastInst::CreateIntegerCast(Shift, CI.getType(), true); + } } } diff --git a/test/Transforms/InstCombine/cast.ll b/test/Transforms/InstCombine/cast.ll index 2f15617e065..486a617097e 100644 --- a/test/Transforms/InstCombine/cast.ll +++ b/test/Transforms/InstCombine/cast.ll @@ -1513,8 +1513,9 @@ define i4 @pr33078_3(i8 %A) { define i8 @pr33078_4(i3 %x) { ; Don't turn this in an `ashr`. This was getting miscompiled ; CHECK-LABEL: @pr33078_4( -; CHECK-NEXT: [[C:%.*]] = ashr i3 %x, 2 -; CHECK-NEXT: [[B:%.*]] = sext i3 [[C]] to i8 +; 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]] %B = sext i3 %x to i16 %C = lshr i16 %B, 13