llvm dev - Jan 2016 - Return value from TargetLowering::LowerOperation?

Hi,

I'm a litle bit puzzled by the TargetLowering::LowerOperation function, 
and what different callers of this function assumes about the returned 
value.

In several places it seems like it is assumed that LowerOperation can 
return three kinds of values:
* Something completely new.
* SDValue()
* The same SDValue as LowerOperation was called on.

However in some places, e.g. in TargetLowering::LowerOperationWrapper, 
it seems like it is assumed only the first two of the above cases can 
happen because there we do:

   SDValue Res = LowerOperation(SDValue(N, 0), DAG);
   if (Res.getNode())
     Results.push_back(Res);

So, when LowerOperationWrapper is called from 
DAGTypeLegalizer::CustomLowerNode, and my target's LowerOperation 
returns the same SDValue as it was called on since we don't want to do 
anything special with this particular SDValue, then we get an assert 
"Potential legalization loop!" when DAGTypeLegalizer::CustomLowerNode 
tries to replace the SDValue with itself.

So in some cases it's ok for LowerOperation to return the same SDValue 
it was called on, and sometimes it's not?

Can anyone shed some light on this?

Thanks,
Mikael

On Fri, Jan 22, 2016 at 01:58:49PM +0100, Mikael Holmén via llvm-dev
wrote:
> Hi,
> 
> I'm a litle bit puzzled by the TargetLowering::LowerOperation function,
> and what different callers of this function assumes about the returned 
> value.
> 
SelectionDAGLegalize::LegalizeOp() is your best reference for this.
> In several places it seems like it is assumed that LowerOperation can 
> return three kinds of values:
> * Something completely new.
> * SDValue()
This tells the legalizer to treat the node as if it were marked Expand.
For loads and stores it tells the legalizer to treat the node as if
it were marked Promote.
> * The same SDValue as LowerOperation was called on.
This tells the legalizer that the input node is legal.
> 
> However in some places, e.g. in TargetLowering::LowerOperationWrapper, 
> it seems like it is assumed only the first two of the above cases can 
> happen because there we do:
> 
>    SDValue Res = LowerOperation(SDValue(N, 0), DAG);
>    if (Res.getNode())
>      Results.push_back(Res);
> 
> So, when LowerOperationWrapper is called from 
> DAGTypeLegalizer::CustomLowerNode, and my target's LowerOperation 
> returns the same SDValue as it was called on since we don't want to do 
> anything special with this particular SDValue, then we get an assert 
> "Potential legalization loop!" when
DAGTypeLegalizer::CustomLowerNode
> tries to replace the SDValue with itself.
> 
I think this error can only happen during type legalization, so my guess
is that you are returning a node that has an illegal type.  Can you
provide more information about the node this is failing with?

-Tom
> So in some cases it's ok for LowerOperation to return the same SDValue 
> it was called on, and sometimes it's not?
> 
> Can anyone shed some light on this?
> 


> Thanks,
> Mikael
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

> On Jan 22, 2016, at 08:02, Tom Stellard via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
>> * The same SDValue as LowerOperation was called on.
> 
> This tells the legalizer that the input node is legal.
I thought SDValue() does this. The same op is supposed to defer to the generic
legalization
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160122/6102e61b/attachment.html>

Hi,

On 01/22/2016 05:02 PM, Tom Stellard wrote:
> On Fri, Jan 22, 2016 at 01:58:49PM +0100, Mikael Holmén via llvm-dev wrote:
>> Hi,
>>
>> I'm a litle bit puzzled by the TargetLowering::LowerOperation
function,
>> and what different callers of this function assumes about the returned
>> value.
>>
> SelectionDAGLegalize::LegalizeOp() is your best reference for this.
>
>> In several places it seems like it is assumed that LowerOperation can
>> return three kinds of values:
>> * Something completely new.
>> * SDValue()
>
> This tells the legalizer to treat the node as if it were marked Expand.
> For loads and stores it tells the legalizer to treat the node as if
> it were marked Promote.
>
>> * The same SDValue as LowerOperation was called on.
>
> This tells the legalizer that the input node is legal.
>
>>
>> However in some places, e.g. in TargetLowering::LowerOperationWrapper,
>> it seems like it is assumed only the first two of the above cases can
>> happen because there we do:
>>
>>     SDValue Res = LowerOperation(SDValue(N, 0), DAG);
>>     if (Res.getNode())
>>       Results.push_back(Res);
>>
>> So, when LowerOperationWrapper is called from
>> DAGTypeLegalizer::CustomLowerNode, and my target's LowerOperation
>> returns the same SDValue as it was called on since we don't want to
do
>> anything special with this particular SDValue, then we get an assert
>> "Potential legalization loop!" when
DAGTypeLegalizer::CustomLowerNode
>> tries to replace the SDValue with itself.
>>
>
> I think this error can only happen during type legalization, so my guess
> is that you are returning a node that has an illegal type.  Can you
> provide more information about the node this is failing with?
On my target v2i16, v4i16, v2i32, v4i32, v2f32, v4f32 are legal and all 
other vector types are not.

Vectors of i16 are a bit special and we need to custom lower bitcasts 
to/from them. Therefore we do

     setOperationAction(ISD::BITCAST, VT, Custom);

on all MVT:s, and in our target's LowerOperation/LowerBitcast we 
specifically handle when the source or target type is v2i16 or v4i16, 
and for other cases we just return the input SDValue and let the 
"normal" code handle it in whatever way it see fits.

In this particular case, when it crashes, we have a bitcast from v2i64 
to v4i32:

t70: v4i32 = bitcast t27

and t27 is

t27: v2i64 = or t15, t26

so it's a bitcast from v2i64 to v4i32 that we don't want to do anything 
special with so we return the input SDValue which 
TargetLowering::LowerOperationWrapper returns back to 
DAGTypeLegalizer::CustomLowerNode and then we get the assert in 
DAGTypeLegalizer::ReplaceValueWith.

Thanks,
Mikael
>
> -Tom
>
>> So in some cases it's ok for LowerOperation to return the same
SDValue
>> it was called on, and sometimes it's not?
>>
>> Can anyone shed some light on this?
>>
>
>
>
>
>> Thanks,
>> Mikael
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>