[Patch] Teach InstCombine to work with smaller legal types in icmp (shl %v, C1), C2

Duncan Sands baldrick at free.fr
Thu Feb 14 12:56:50 PST 2013


Hi Arnaud,

> @@ -1331,6 +1331,24 @@ Instruction *InstCombiner::visitICmpInstWithInstAndIntCst(ICmpInst &ICI,
>        return new ICmpInst(TrueIfSigned ? ICmpInst::ICMP_NE : ICmpInst::ICMP_EQ,
>                            And, Constant::getNullValue(And->getType()));
>      }
> +
> +    // Transform (icmp pred iM (shl iM %v, N), CI)
> +    // -> (icmp pred i(M-N) (trunc %v iM to i(N-N)), (asr CI, N))
> +    // Transform the shl to a trunc if (asr CI, N) has no loss.
> +    // This enables to get rid of the shift in favor of a trunc which can be
> +    // free on the target. It has the additional benefit of comparing to a
> +    // smaller constant, which will be target friendly.
> +    if (ShAmt->getZExtValue() &&

you can use !ShAmt->isZero().  I don't like getZExtValue() because it will crash
if the value doesn't fit in 64 bits, which is theoretically possible.  You can
also use !RHS->isNullValue().  Also, won't ShAmt->getZExtValue() crash if this
is a vector shift?

Alternatively, you could do:
   unsigned BitWidth = RHS->getType()->getPrimitiveSizeInBits();
   unsigned Amt = ShAmt->getLimitedValue(BitWidth);
Then use Amt rather than ShAmt->getZExtValue() everywhere.  This is valid since
if the shift amount is bigger than the bitwidth then the shift represents
undefined behaviour.  Maybe better is to use BitWidth-1, since that is still OK
and avoids the possible DestTypeSize being zero problem described below.

> +        RHS->getValue().countTrailingZeros() >= ShAmt->getZExtValue()) {


> +      unsigned DestTypeSize =
> +        LHSI->getType()->getPrimitiveSizeInBits() - ShAmt->getZExtValue();

If RHS is zero (so countTrailingZeros is equal to the bitwidth), and
ShAmt->getZExtValue() is equal to the bitwidth, then DestTypeSize will
be zero and you will crash below.  Also, since everything so far was in
terms of RHS, it seems odd to pass to LHSI, you could use RHS->getType().

> +      Type *NTy = IntegerType::get(LHSI->getContext(), DestTypeSize);
> +      Constant *NCI = ConstantExpr::getAShr(RHS, ShAmt);
> +      return new ICmpInst(ICI.getPredicate(),
> +                          Builder->CreateTrunc(LHSI->getOperand(0), NTy),
> +                          ConstantExpr::getTrunc(NCI, NTy));
> +    }
> +
>      break;
>    }
>

Ciao, Duncan.



More information about the llvm-commits mailing list