[PATCH] Fix PR16360

[Duncan Sands]

Hi Michael,

On 19/06/13 19:14, Michael Liao wrote:
> Hi,
>
> The attached patch fixes PR16360. In DAG combining, when (srl (anyextend
> x), c) is folded into (anyextend (srl x, c)), the high bits are not
> cleared. That makes such transformation too aggressive and generates
> wrong code. The fix will 'and' a constant mask after 'anyextend', i.e.
> folding from (srl (anyextend x), c) to (and (anyextend (srl x, c)),
> mask).

is this transformation actually profitable?

> --- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> +++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> @@ -3915,8 +3915,7 @@ SDValue DAGCombiner::visitSRL(SDNode *N) {
>                         DAG.getConstant(~0ULL >> ShAmt, VT));
>    }
>
> -
> -  // fold (srl (anyextend x), c) -> (anyextend (srl x, c))
> +  // fold (srl (anyextend x), c) -> (and (anyextend (srl x, c)), mask)
>    if (N1C && N0.getOpcode() == ISD::ANY_EXTEND) {
>      // Shifting in all undef bits?
>      EVT SmallVT = N0.getOperand(0).getValueType();
> @@ -3929,7 +3928,10 @@ SDValue DAGCombiner::visitSRL(SDNode *N) {
>                                         N0.getOperand(0),
>                            DAG.getConstant(ShiftAmt, getShiftAmountTy(SmallVT)));
>        AddToWorkList(SmallShift.getNode());
> -      return DAG.getNode(ISD::ANY_EXTEND, SDLoc(N), VT, SmallShift);
> +      APInt Mask = APInt::getAllOnesValue(VT.getSizeInBits()).lshr(ShiftAmt);

Here you could use APInt::getLowBitsSet.  There is a danger if ShiftAmt is
larger than the width of the integer, since the method will assert, but if so
then the result of the shift is "undef" anyway so I'm expecting that this case
has already been taken care of earlier somewhere.  You might want to add a test
case for it though.

Ciao, Duncan.

> +      return DAG.getNode(ISD::AND, SDLoc(N), VT,
> +                         DAG.getNode(ISD::ANY_EXTEND, SDLoc(N), VT, SmallShift),
> +                         DAG.getConstant(Mask, VT));
>      }
>    }
>
> diff --git a/test/CodeGen/X86/pr16360.ll b/test/CodeGen/X86/pr16360.ll
> new file mode 100644
> index 0000000..97b151b
> --- /dev/null
> +++ b/test/CodeGen/X86/pr16360.ll
> @@ -0,0 +1,16 @@
> +; RUN: llc < %s -march=x86 | FileCheck %s
> +
> +define i64 @foo(i32 %sum) {
> +entry:
> +  %conv = sext i32 %sum to i64
> +  %shr = lshr i64 %conv, 2
> +  %or = or i64 4611686018360279040, %shr
> +  ret i64 %or
> +}
> +
> +; CHECK: foo
> +; CHECK: shrl $2
> +; CHECK: orl $-67108864
> +; CHECK-NOT: movl $-1
> +; CHECK: movl $1073741823
> +; CHECK: ret
> -- 1.7.9.5
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>