[llvm-commits] [llvm] r51696 - in /llvm/trunk: lib/CodeGen/SelectionDAG/SelectionDAG.cpp lib/Target/X86/X86ISelLowering.cpp test/CodeGen/X86/memmove-4.ll

[Chris Lattner]

On May 30, 2008, at 6:46 PM, Dan Gohman wrote:
> On May 29, 2008, at 6:49 PM, Chris Lattner wrote:
>>>
>>> +  std::string Str;
>>
>> Str is dead.
>
> Done.

Thanks!

>>>
>>> +  SmallVector<SDOperand, 8> OutChains;
>>> +  unsigned NumMemOps = MemOps.size();
>>> +  for (unsigned i = 0; i < NumMemOps; i++) {
>>> +    MVT::ValueType VT = MemOps[i];
>>> +    unsigned VTSize = MVT::getSizeInBits(VT) / 8;
>>> +    SDOperand Value, Store;
>>> +
>>> +    Value = DAG.getLoad(VT, Chain,
>>> +                        getMemBasePlusOffset(Src, SrcOff, DAG),
>>> +                        SrcSV, SrcSVOff + SrcOff, false, Align);
>>> +    LoadValues.push_back(Value);
>>> +    LoadChains.push_back(Value.getValue(1));
>>> +    SrcOff += VTSize;
>>> +  }
>>
>> NumMemOps can never be zero here, right?  How about an assert?
>
> What about a memmove of size 0? It's true that we have multiple
> optimizations that will zap such a thing, but we don't want llc
> to abort if those optimizations aren't run.

I completely agree.  However, if NumMemOps can be zero, the code won't  
work (because you'll make a TF with zero inputs).  This should be an  
assert and handled explicitly or checked in the caller.

-Chris