[llvm] r227397 - Add nullptr checks for TargetSelectionDAGInfo in SelectionDAG.

Manuel Jacob me at manueljacob.de
Wed Jan 28 17:01:40 PST 2015


On 2015-01-29 01:22, David Blaikie wrote:
> Any test coverage?

I hit a nullptr dereference when passing a variable-sized memcpy to the 
R600 backend, which is illegal anyway, so I couldn't think of a sensible 
test case.  Also there doesn't seem to be a way to force TSI to be 
nullptr.  It would be possible to add a test case for a target that 
allows variable-sized memcpys and doesn't provide TargetSelectionDAGInfo 
currently.  Is that a good test case, considering that this target could 
start to provide TargetSelectionDAGInfo at some point in the future, 
making this test not test anything?  If yes, should it test all three of 
memcpy, memset and memmove?

> On Wed, Jan 28, 2015 at 3:50 PM, Manuel Jacob <me at manueljacob.de> 
> wrote:
> 
>> Author: mjacob
>> Date: Wed Jan 28 17:50:40 2015
>> New Revision: 227397
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=227397&view=rev
>> Log:
>> Add nullptr checks for TargetSelectionDAGInfo in SelectionDAG.
>> 
>> TSI is not guaranteed be non-null in SelectionDAG.
>> 
>> Modified:
>>     llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> 
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=227397&r1=227396&r2=227397&view=diff
>> 
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Wed Jan 28
>> 17:50:40 2015
>> @@ -4272,11 +4272,13 @@ SDValue SelectionDAG::getMemcpy(SDValue
>> 
>>    // Then check to see if we should lower the memcpy with 
>> target-specific
>>    // code. If the target chooses to do this, this is the next best.
>> -  SDValue Result =
>> -      TSI->EmitTargetCodeForMemcpy(*this, dl, Chain, Dst, Src, Size,
>> Align,
>> -                                   isVol, AlwaysInline, DstPtrInfo,
>> SrcPtrInfo);
>> -  if (Result.getNode())
>> -    return Result;
>> +  if (TSI) {
>> +    SDValue Result = TSI->EmitTargetCodeForMemcpy(
>> +        *this, dl, Chain, Dst, Src, Size, Align, isVol, AlwaysInline,
>> +        DstPtrInfo, SrcPtrInfo);
>> +    if (Result.getNode())
>> +      return Result;
>> +  }
>> 
>>    // If we really need inline code and the target declined to provide 
>> it,
>>    // use a (potentially long) sequence of loads and stores.
>> @@ -4338,10 +4340,12 @@ SDValue SelectionDAG::getMemmove(SDValue
>> 
>>    // Then check to see if we should lower the memmove with 
>> target-specific
>>    // code. If the target chooses to do this, this is the next best.
>> -  SDValue Result = TSI->EmitTargetCodeForMemmove(
>> -      *this, dl, Chain, Dst, Src, Size, Align, isVol, DstPtrInfo,
>> SrcPtrInfo);
>> -  if (Result.getNode())
>> -    return Result;
>> +  if (TSI) {
>> +    SDValue Result = TSI->EmitTargetCodeForMemmove(
>> +        *this, dl, Chain, Dst, Src, Size, Align, isVol, DstPtrInfo,
>> SrcPtrInfo);
>> +    if (Result.getNode())
>> +      return Result;
>> +  }
>> 
>>    // FIXME: If the memmove is volatile, lowering it to plain libc 
>> memmove
>> may
>>    // not be safe.  See memcpy above for more details.
>> @@ -4390,10 +4394,12 @@ SDValue SelectionDAG::getMemset(SDValue
>> 
>>    // Then check to see if we should lower the memset with 
>> target-specific
>>    // code. If the target chooses to do this, this is the next best.
>> -  SDValue Result = TSI->EmitTargetCodeForMemset(*this, dl, Chain, 
>> Dst,
>> Src,
>> -                                                Size, Align, isVol,
>> DstPtrInfo);
>> -  if (Result.getNode())
>> -    return Result;
>> +  if (TSI) {
>> +    SDValue Result = TSI->EmitTargetCodeForMemset(
>> +        *this, dl, Chain, Dst, Src, Size, Align, isVol, DstPtrInfo);
>> +    if (Result.getNode())
>> +      return Result;
>> +  }
>> 
>>    // Emit a library call.
>>    Type *IntPtrTy = 
>> TLI->getDataLayout()->getIntPtrType(*getContext());
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 




More information about the llvm-commits mailing list