Enable GT_CALL with long ret types for x86 RyuJIT#5890
Conversation
|
@CarolEidt @sivarv PTAL |
| GenTreePtr* treePtr = nullptr; | ||
| parent = tree->gtGetParent(&treePtr); | ||
|
|
||
| GenTreeStmt* asgStmt = comp->fgInsertEmbeddedFormTemp(treePtr); |
There was a problem hiding this comment.
@CarolEidt can new asgStmt become a top-level stmt since the check above is "if (parent !=nullptr)" that is GT_CALL node has a parent? The case where GT_CALL node is a top-level stmt without any parent, there is no need to force IR to be var=call. Did I mis-understood anything?
There was a problem hiding this comment.
That is a good point. So I don't think a TODO is required here. A comment would be useful to the effect that, since this is known to have a parent, we needn't worry about creating a new top-level statement.
|
Looks Good |
| unsigned retRegCount = retTypeDesc->GetReturnRegCount(); | ||
| // must be a long returned in two registers | ||
| assert(retRegCount == 2); | ||
|
|
There was a problem hiding this comment.
Function header comment needs to be updated, since this routine is just initializing return type descriptor of a call node. Also should we rename this method to be representative of its new functionality (e.g. impInitCallReturnTypeDesc())
Note that right now impFixupCallStructReturn() is also initializing return type desc of a call node for unix amd64. @briansull would need to add similar code for Arm64 case. In future when we add support for arrm32 this routine would come handy.
There was a problem hiding this comment.
updated the function name and header. Should we also change the logic of impFixupCallStructReturn to call this function (and make it available to more than just TARGET_X86)?
There was a problem hiding this comment.
Yes, I would like that change. But I am equally fine if you don't want to club that change with this PR.
michellemcdaniel
force-pushed
the
x86LongCallFinal
branch
from
aa9980b to
32e4a54
Compare
| GenTree* parent = data->parent; | ||
| if (parent != nullptr) | ||
| { | ||
| // We do not need to force var = call() if the call is a top-level node. |
There was a problem hiding this comment.
Please add this comment above 388. The fact that we are forcing IR to be var=call only if GT_CALL node has a parent indicates that GT_CALL node is not a top-level stmnt.
|
Looks good. |
|
LGTM |
michellemcdaniel
force-pushed
the
x86LongCallFinal
branch
from
32e4a54 to
9a1a536
Compare
1) Enables genMultiRegCallStoreToLocal for x86 RyuJIT. Forces long return types to be stored to the stack after returning from a call. 2) Update lvaPromoteLongVars to not promote a long if it is a multi reg arg or ret. 3) Adds NYI for call arguments that are longs and contain adds or subtracts. We are currently producing the wrong order of operations so that we can push the argument immediately after performing the arithmetic. We will do the hi adc/sbb before the carry bit has been set by doing the lo operation. 4) Adds an NYI for morphing a node into a call node if the call will have a long return type. 5) Moves the logic for forcing var = call() for calls with long return types to lower::DecomposeNode().
michellemcdaniel
force-pushed
the
x86LongCallFinal
branch
from
9a1a536 to
9237cec
Compare
|
@dotnet-bot test this please |
Enable GT_CALL with long ret types for x86 RyuJIT Commit migrated from dotnet/coreclr@26d4a8c
6 participants
Enables genMultiRegCallStoreToLocal for x86 RyuJIT. Forces long return
types to be stored to the stack after returning from a call.
Update lvaPromoteLongVars to not promote a long if it is a multi reg
arg or ret.
Adds NYI for call arguments that are longs and contain adds or
subtracts. We are currently producing the wrong order of operations so
that we can push the argument immediately after performing the arithmetic.
We will do the hi adc/sbb before the carry bit has been set by doing the
lo operation.
Adds an NYI for morphing a node into a call node if the call will have
a long return type.
Moves the logic for forcing var = call() for calls with long return
types to lower::DecomposeNode().