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

[Eli Friedman]

On Sat, Sep 24, 2011 at 12:17 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> 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?

Hi Nadav,

Did you ever end up updating this patch?

-Eli