[LLVMdev] Question regarding ReplaceValueWith and ReplaceNodeResults

[Duncan Sands]

Hi Pranav,

>> as well as what Eli said, ReplaceNodeResults requires (IIRC) the new node
> to
>> have the same type as the old node, which doesn't seem to be the case
>> here.
>
> Are you sure ? I see ReplaceNodeResults being called from functions such as
> CustomWidenLowerNode and CustomLowerNode.
> In the former, we are clearly expecting a change in type, aren't we?

yes, but in CustomLowerNode the type shouldn't change.  It is bad that the
same method (ReplaceNodeResults) is being called with different requirements.
CustomLowerNode was the original method, and whoever added CustomWidenLowerNode
was wrong to reuse ReplaceNodeResults like this IMO.

Note that the documentation for ReplaceNodeResults is explicit about how the
type should not change:

   /// ReplaceNodeResults - This callback is invoked when a node result type is
   /// illegal for the target, and the operation was registered to use 'custom'
   /// lowering for that result type.  The target places new result values for
   /// the node in Results (their number and types must exactly match those of
   /// the original return values of the node), or leaves Results empty, which
   /// indicates that the node is not to be custom lowered after all.
   ///
   /// If the target has no operations that require custom lowering, it need not
   /// implement this.  The default implementation aborts.

  Even in
> the latter, ReplaceNodeResults  is called only when the type of the result
> is not legal.

Yes, and?  If the result type is i1 then the target code gets the opportunity
to replace it with some other node with type i1, which probably means a target
specific node with type i8 followed by a truncate to i1.

Ciao, Duncan.

> Otherwise, the latter calls LowerOperationWrapper.


>
> Thanks,
> Pranav
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>