Question regarding ReplaceValueWith and ReplaceNodeResults

Hi Duncan,

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? Even in
the latter, ReplaceNodeResults is called only when the type of the result
is not legal.
Otherwise, the latter calls LowerOperationWrapper.

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

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.

Hi Duncan,

Thanks for the information. Another question inline below.

   /// 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.

Right. Now, we have a unique case in Hexagon, wherein we have some
intrinsics that return i8.
"i8" itself is not a supported type in general on hexagon, except for
instructions that return predicates. These 8bit results from intrinsics are
stored in predicate registers.
Typically, such intrinsics are for comparisons and bit-field testing /
setting /clearing. There are no 8 bit registers for arithmetic though.

The problem is that if we add "i8" to the PredRegsClass then Isel lowering
thinks "i8" is a legal type on Hexagon, which is not necessarily the case.
We decided to fake the PredRegsClass as taking "i32" too and then use
instruction patterns (PredRegs:$src1 vs IntRegs:$src1) to disambiguate
between the use of PredRegs and IntRegs (32 bit GPR).

So, the intrinsic in question is defined to return "i8" (llvm) or 'c' (in
clang) and we hope to use the type legalizer to promote the result to 'i32'.
This way we can then write a pattern for i32 that uses PredRegs.
But, the problem is I cannot setOperationAction for INTRINSIC_WO_CHAIN to
promote because I do not want to do that for all intrinsics. So, I set it to
"custom" and tried to, as it turns out now, quite wrongly, use
ReplaceNodeResults. How do I get the type of the result to be promoted to
i32 then ? Any suggestions would be much appreciated.

Thanks,
Pranav

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