returning from LowerOperation()

Hi Eli,

I would like to clarify generally what the difference is between returning SDValue() and Op (input argument unchanged) from LowerOperation()?

My understanding is that returning SDValue() means that Target gives up, and the common code is supposed to handle it. Returning Op, the unchanged argument, means that the Target is happy with the node as it is, and the common code can move on to something else.

Are there any exceptions to this? Is this commented on anywhere? Are there cases where the target should not return SDValue()?

Thanks,

Jonas

Hi Eli,

I would like to clarify generally what the difference is between returning SDValue() and Op (input argument unchanged) from LowerOperation()?

My understanding is that returning SDValue() means that Target gives up, and the common code is supposed to handle it. Returning Op, the unchanged argument, means that the Target is happy with the node as it is, and the common code can move on to something else.

This is right.

Are there any exceptions to this? Is this commented on anywhere? Are there cases where the target should not return SDValue()?

No exceptions to this. Not sure if it's described anywhere off the top of my head; if there isn't a comment describing this on the declaration of LowerOperation or LowerOperationWrapper, it would probably be good to add one. Returning SDValue() should be "fine" in all cases (it might lead to a fatal error if we can't actually lower the operation).

-Eli

This sounds backwards. Returning SDValue() means the node should be treated as legal. Returning the original operation should hit the expand path.

-Matt

No, I think that was correct. The code in lib/CodeGen/SelectionDAG/LegalizeDAG.cpp reads: switch (Action) { case TargetLowering::Legal: return; case TargetLowering::Custom: { // FIXME: The handling for custom lowering with multiple results is // a complete mess. if (SDValue Res = TLI.LowerOperation(SDValue(Node, 0), DAG)) { if (!(Res.getNode() != Node || Res.getResNo() != 0)) return; … ReplaceNode(Node, ResultVals.data()); return; } LLVM_FALLTHROUGH; } case TargetLowering::Expand: if (ExpandNode(Node)) return; So, if you return an SDValue() then it will Expand. If you return the original node, that is equivalent to Legal. Otherwise, you’re requesting a replacement. The logic for loads/stores and vectors is handled separately (but is similar). -Hal

Hi all,

thanks!

Not sure if it’s described anywhere off the top of my head; if there isn’t a comment describing this on the declaration of LowerOperation or LowerOperationWrapper, it would probably be good to add one. Returning SDValue() should be “fine” in all cases (it might lead to a fatal error if we can’t actually lower the operation).

Does this sound good?

diff --git a/include/llvm/Target/TargetLowering.h b/include/llvm/Target/TargetLowering.h
index 01c5c51..41edcc6 100644
--- a/include/llvm/Target/TargetLowering.h
+++ b/include/llvm/Target/TargetLowering.h
@@ -2830,7 +2830,8 @@ public:
/// target, which are registered to use 'custom' lowering, and whose defined
/// values are all legal. If the target has no operations that require custom
/// lowering, it need not implement this. The default implementation of this
- /// aborts.
+ /// aborts. Returning SDValue() is ok and signals that the input node
+ /// should be further handled by the caller instead.
virtual SDValue LowerOperation(SDValue Op, SelectionDAG &DAG) const;
``
/// This callback is invoked when a node result type is illegal for the

/ Jonas

Why don’t we just say, “Returning SDValue() indicates that the input node should be expanded.”? -Hal