[llvm-commits] Optimization for SIGN_EXTEND on AVX and AVX2 - please review

[Duncan Sands]

Hi Elena,

On 15/01/13 12:47, Demikhovsky, Elena wrote:
> I optimized the following SEXT pairs:
> v8i8 -> v8i64
> v8i8->v8i32
> v4i8 ->v4i64
> v4i16->v4i64

> Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> ===================================================================
> --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp	(revision 172422)
> +++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp	(working copy)
> @@ -4298,12 +4298,13 @@
>    if (isa<ConstantSDNode>(N0))
>      return DAG.getNode(ISD::SIGN_EXTEND, N->getDebugLoc(), VT, N0);
>
> -  // fold (sext (sext x)) -> (sext x)
> -  // fold (sext (aext x)) -> (sext x)
> -  if (N0.getOpcode() == ISD::SIGN_EXTEND || N0.getOpcode() == ISD::ANY_EXTEND)
> -    return DAG.getNode(ISD::SIGN_EXTEND, N->getDebugLoc(), VT,
> -                       N0.getOperand(0));
> -
> +  if (Level >= AfterLegalizeTypes) {
> +    // fold (sext (sext x)) -> (sext x)
> +    // fold (sext (aext x)) -> (sext x)
> +    if (N0.getOpcode() == ISD::SIGN_EXTEND || N0.getOpcode() == ISD::ANY_EXTEND)
> +      return DAG.getNode(ISD::SIGN_EXTEND, N->getDebugLoc(), VT,
> +                         N0.getOperand(0));
> +  }

why is this needed?  As the transform doesn't introduce any illegal types that
weren't there already it should be safe to perform at any level.

If you are worried that it might create an illegal operation, then that is what
you should check for, not the Level.

>    if (N0.getOpcode() == ISD::TRUNCATE) {
>      // fold (sext (truncate (load x))) -> (sext (smaller load x))
>      // fold (sext (truncate (srl (load x), c))) -> (sext (smaller load (x+c/n)))

...

> --- lib/CodeGen/SelectionDAG/SelectionDAG.cpp	(revision 172422)
> +++ lib/CodeGen/SelectionDAG/SelectionDAG.cpp	(working copy)
> @@ -2554,9 +2554,7 @@
>              VT.getVectorNumElements() ==
>              Operand.getValueType().getVectorNumElements()) &&
>             "Vector element count mismatch!");
> -    if (OpOpcode == ISD::SIGN_EXTEND || OpOpcode == ISD::ZERO_EXTEND)
> -      return getNode(OpOpcode, DL, VT, Operand.getNode()->getOperand(0));
> -    else if (OpOpcode == ISD::UNDEF)
> +    if (OpOpcode == ISD::UNDEF)
>        // sext(undef) = 0, because the top bits will all be the same.
>        return getConstant(0, VT);
>      break;

Likewise, why are you getting rid of this folding?

Ciao, Duncan.