[llvm-commits] [PATCH - ]Bug 10747 - miscompiled extractelement instruction

[Eli Friedman]

On Sat, Sep 24, 2011 at 2:07 AM, Rotem, Nadav <nadav.rotem at intel.com> wrote:
> Hi Eli,
>
> Please review the patch below, which addresses pr10747.
>
> Thanks,
> Nadav
>
> Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> ===================================================================
> --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp  (revision 140246)
> +++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp  (working copy)
> @@ -6929,9 +6929,15 @@
>                             DAG.getConstant(PtrOff, PtrType));
>      }
>
> -    return DAG.getLoad(LVT, N->getDebugLoc(), LN0->getChain(), NewPtr,
> -                       LN0->getPointerInfo().getWithOffset(PtrOff),
> -                       LN0->isVolatile(), LN0->isNonTemporal(), Align);
> +    SDValue Load =  DAG.getLoad(LVT, N->getDebugLoc(), LN0->getChain(),
> +                           NewPtr,
> +                           LN0->getPointerInfo().getWithOffset(PtrOff),
> +                           LN0->isVolatile(), LN0->isNonTemporal(), Align);
> +
> +    WorkListRemover DeadNodes(*this);
> +    DAG.ReplaceAllUsesOfValueWith(SDValue(LN0,1), Load.getValue(1),
> &DeadNodes);
> +    DAG.ReplaceAllUsesOfValueWith(SDValue(N, 0), Load.getValue(0),
> &DeadNodes);
> +    return SDValue(N, 0);

Close... I think you have to explicitly delete LN0.  (And you don't
actually need to explcitly RAUW N; just retrn LN0.)

>
>    return SDValue();
>
> Index: test/CodeGen/X86/vec_extract-sse4.ll
> ===================================================================
> --- test/CodeGen/X86/vec_extract-sse4.ll        (revision 140246)
> +++ test/CodeGen/X86/vec_extract-sse4.ll        (working copy)
> @@ -2,7 +2,7 @@
> ; RUN: not grep extractps   %t
> ; RUN: not grep pextrd      %t
> ; RUN: not grep pshufd  %t
> -; RUN: grep movss   %t | count 2
> +; RUN: not grep movss   %t
>
> define void @t1(float* %R, <4 x float>* %P1) nounwind {
>         %X = load <4 x float>* %P1

Mind fixing this to use FileCheck while you're here?

-Eli