From da1c122da5e1a86c8c2bf20e47a43f5c8fefc4e2 Mon Sep 17 00:00:00 2001 From: Owen Anderson Date: Tue, 11 Jan 2011 00:36:45 +0000 Subject: [PATCH] Fix a random missed optimization by making InstCombine more aggressive when determining which bits are demanded by a comparison against a constant. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@123203 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/README.txt | 15 ------- .../InstCombine/InstCombineCompares.cpp | 42 ++++++++++++++++++- test/Transforms/InstCombine/icmp.ll | 17 ++++++++ 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/lib/Target/README.txt b/lib/Target/README.txt index dfa00b8c951..194a19219cb 100644 --- a/lib/Target/README.txt +++ b/lib/Target/README.txt @@ -1627,21 +1627,6 @@ int bar() { return foo("abcd"); } //===---------------------------------------------------------------------===// -InstCombine should use SimplifyDemandedBits to remove the or instruction: - -define i1 @test(i8 %x, i8 %y) { - %A = or i8 %x, 1 - %B = icmp ugt i8 %A, 3 - ret i1 %B -} - -Currently instcombine calls SimplifyDemandedBits with either all bits or just -the sign bit, if the comparison is obviously a sign test. In this case, we only -need all but the bottom two bits from %A, and if we gave that mask to SDB it -would delete the or instruction for us. - -//===---------------------------------------------------------------------===// - functionattrs doesn't know much about memcpy/memset. This function should be marked readnone rather than readonly, since it only twiddles local memory, but functionattrs doesn't handle memset/memcpy/memmove aggressively: diff --git a/lib/Transforms/InstCombine/InstCombineCompares.cpp b/lib/Transforms/InstCombine/InstCombineCompares.cpp index 6c6d26c81c7..985d9129b3e 100644 --- a/lib/Transforms/InstCombine/InstCombineCompares.cpp +++ b/lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -1693,6 +1693,45 @@ static Instruction *ProcessUAddIdiom(Instruction &I, Value *OrigAddV, return ExtractValueInst::Create(Call, 1, "uadd.overflow"); } +// DemandedBitsLHSMask - When performing a comparison against a constant, +// it is possible that not all the bits in the LHS are demanded. This helper +// method computes the mask that IS demanded. +static APInt DemandedBitsLHSMask(ICmpInst &I, + unsigned BitWidth, bool isSignCheck) { + if (isSignCheck) + return APInt::getSignBit(BitWidth); + + ConstantInt *CI = dyn_cast(I.getOperand(1)); + if (!CI) return APInt::getAllOnesValue(BitWidth); + + APInt RHS = CI->getValue(); + APInt Mask(BitWidth, 0); + + switch (I.getPredicate()) { + // For a UGT comparison, we don't care about any bits that + // correspond to the trailing ones of the comparand. The value of these + // bits doesn't impact the outcome of the comparison, because any value + // greater than the RHS must differ in a bit higher than these due to carry. + case ICmpInst::ICMP_UGT: { + unsigned trailingOnes = RHS.countTrailingOnes(); + APInt lowBitsSet = APInt::getLowBitsSet(BitWidth, trailingOnes); + return ~lowBitsSet; + } + + // Similarly, for a ULT comparison, we don't care about the trailing zeros. + // Any value less than the RHS must differ in a higher bit because of carries. + case ICmpInst::ICMP_ULT: { + unsigned trailingZeros = RHS.countTrailingZeros(); + APInt lowBitsSet = APInt::getLowBitsSet(BitWidth, trailingZeros); + return ~lowBitsSet; + } + + default: + return APInt::getAllOnesValue(BitWidth); + } + + return Mask; +} Instruction *InstCombiner::visitICmpInst(ICmpInst &I) { bool Changed = false; @@ -1830,8 +1869,7 @@ Instruction *InstCombiner::visitICmpInst(ICmpInst &I) { APInt Op1KnownZero(BitWidth, 0), Op1KnownOne(BitWidth, 0); if (SimplifyDemandedBits(I.getOperandUse(0), - isSignBit ? APInt::getSignBit(BitWidth) - : APInt::getAllOnesValue(BitWidth), + DemandedBitsLHSMask(I, BitWidth, isSignBit), Op0KnownZero, Op0KnownOne, 0)) return &I; if (SimplifyDemandedBits(I.getOperandUse(1), diff --git a/test/Transforms/InstCombine/icmp.ll b/test/Transforms/InstCombine/icmp.ll index e33463dbe50..8a0b8ff93b5 100644 --- a/test/Transforms/InstCombine/icmp.ll +++ b/test/Transforms/InstCombine/icmp.ll @@ -192,3 +192,20 @@ define i1 @test20(i32 %x) nounwind { ; CHECK-NEXT: %cmp = icmp eq i32 %x, 3 } +define i1 @test21(i8 %x, i8 %y) { +; CHECK: @test21 +; CHECK-NOT: or i8 +; CHECK: icmp ugt + %A = or i8 %x, 1 + %B = icmp ugt i8 %A, 3 + ret i1 %B +} + +define i1 @test22(i8 %x, i8 %y) { +; CHECK: @test22 +; CHECK-NOT: or i8 +; CHECK: icmp ult + %A = or i8 %x, 1 + %B = icmp ult i8 %A, 4 + ret i1 %B +} -- 2.34.1