[llvm-commits] [patch][pr12251] Add a "range" metadata, use it to remove unnecessary ands

[Duncan Sands]

Hi Rafael,

> The attached patches add a range metadata to llvm and change clang to
> produce it. I think it covers all the use cases in comment 7 on
> pr12251.

> --- a/lib/Analysis/InstructionSimplify.cpp
> +++ b/lib/Analysis/InstructionSimplify.cpp
> @@ -1322,6 +1322,19 @@ static Value *SimplifyAndInst(Value *Op0, Value *Op1, const Query &Q,
>      std::swap(Op0, Op1);
>    }
>
> +  if (ConstantInt *CRHS = dyn_cast<ConstantInt>(Op1)) {
> +    APInt Value = CRHS->getValue();
> +    unsigned Bitwidth = Value.getBitWidth();
> +    APInt WillBeZero = ~Value;
> +    APInt DemandedMask = APInt::getAllOnesValue(Bitwidth);
> +    APInt KnownZero = APInt::getNullValue(Bitwidth);
> +    APInt KnownOne = APInt::getNullValue(Bitwidth);
> +
> +    ComputeMaskedBits(Op0, DemandedMask, KnownZero, KnownOne);
> +    if ((WillBeZero & KnownZero) == WillBeZero)
> +      return Op0;
> +  }

please move this lower down, after the cheap and simple checks.  Probably you
want to put it just before "Try some generic simplifications for associative
operations".  Also, this transform could be more general, for example it doesn't
need Op1 to be a constant.  For each of Op0, Op1 you can get the KnownZero and
KnownOne bits.  If (KnownZero0 | KnownOne1) has all bits set then you can return
Op0, generalizing your transform; if (KnownOne0 | KnownZero1) has all bits set
then you can return Op1.

> --- a/lib/Analysis/ValueTracking.cpp
> +++ b/lib/Analysis/ValueTracking.cpp
> @@ -20,6 +20,7 @@
>  #include "llvm/GlobalAlias.h"
>  #include "llvm/IntrinsicInst.h"
>  #include "llvm/LLVMContext.h"
> +#include "llvm/Metadata.h"
>  #include "llvm/Operator.h"
>  #include "llvm/Target/TargetData.h"
>  #include "llvm/Support/GetElementPtrTypeIterator.h"
> @@ -195,6 +196,26 @@ static void ComputeMaskedBitsMul(Value *Op0, Value *Op1, bool NSW,
>      KnownOne.setBit(BitWidth - 1);
>  }
>
> +static void computeMaskedBitsLoad(MDNode *Ranges, const APInt &Mask,
> +                                  APInt &KnownZero) {
> +  if (!Ranges)
> +    return;
> +
> +  unsigned BitWidth = Mask.getBitWidth();
> +  unsigned NumRanges = Ranges->getNumOperands();
> +  assert(NumRanges >= 1);
> +
> +  // Use the high end of the ranges to find leading zeros.

I think this definition of range is too limited since it doesn't allow for
"wrapping around the top", which is helpful for allowing things like "the
top N bits are all equal to the sign bit.  For example, suppose the type
is i8 and you want to say that the range is -2 .. +1, i.e. that the top 7
bits are always equal to the sign bit.  You could express this the way LLVM's 
ConstantRange class does, using the range 254 .. 1, which means the numbers
254, 255, 0, 1.  Unfortunately the logic you have below \/ would look at the
"high value" of the range, i.e. 1, and think that the top 7 bits have to be
zero, while the fact that the range contains -1 means that in fact no top
bit need be zero.  So it would have to be made a bit more complicated, but
it would be fairly straightforward with the help of the ConstantRange class.

>  /// ComputeMaskedBits - Determine which of the bits specified in Mask are
>  /// known to be either zero or one and return them in the KnownZero/KnownOne
>  /// bit sets.  This code only analyzes bits in Mask, in order to short-circuit
> @@ -315,6 +336,10 @@ void llvm::ComputeMaskedBits(Value *V, const APInt &Mask,
>    APInt KnownZero2(KnownZero), KnownOne2(KnownOne);
>    switch (I->getOpcode()) {
>    default: break;
> +  case Instruction::Load:
> +    computeMaskedBitsLoad(cast<LoadInst>(I)->getMetadata(LLVMContext::MD_range),
> +                          Mask, KnownZero);

You could also estimate KnownOne, for example an i8 type with range 128 .. 255
would have top bit always 1, but probably it is not worth doing this until there
is a use for it.

> +    return;
>    case Instruction::And: {
>      // If either the LHS or the RHS are Zero, the result is zero.
>      ComputeMaskedBits(I->getOperand(1), Mask, KnownZero, KnownOne, TD, Depth+1);

Ciao, Duncan.