-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Handle addressing modes for HW intrinsics #22944
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1333,12 +1333,27 @@ void CodeGen::genConsumeRegs(GenTree* tree) | |
| // Update the life of the lcl var. | ||
| genUpdateLife(tree); | ||
| } | ||
| #endif // _TARGET_XARCH_ | ||
| else if (tree->OperIsInitVal()) | ||
| #ifdef FEATURE_HW_INTRINSICS | ||
| else if (tree->OperIs(GT_HWIntrinsic)) | ||
| { | ||
| genConsumeReg(tree->gtGetOp1()); | ||
| // Only load/store HW intrinsics can be contained (and the address may also be contained). | ||
| HWIntrinsicCategory category = HWIntrinsicInfo::lookupCategory(tree->AsHWIntrinsic()->gtHWIntrinsicId); | ||
| assert((category == HW_Category_MemoryLoad) || (category == HW_Category_MemoryStore)); | ||
| int numArgs = HWIntrinsicInfo::lookupNumArgs(tree->AsHWIntrinsic()); | ||
| genConsumeAddress(tree->gtGetOp1()); | ||
| if (category == HW_Category_MemoryStore) | ||
| { | ||
| assert((numArgs == 2) && !tree->gtGetOp2()->isContained()); | ||
| genConsumeReg(tree->gtGetOp2()); | ||
| } | ||
| else | ||
| { | ||
| assert(numArgs == 1); | ||
| } | ||
| } | ||
| else if (tree->OperIsHWIntrinsic()) | ||
| #endif // FEATURE_HW_INTRINSICS | ||
| #endif // _TARGET_XARCH_ | ||
| else if (tree->OperIsInitVal()) | ||
| { | ||
| genConsumeReg(tree->gtGetOp1()); | ||
| } | ||
|
|
@@ -1368,11 +1383,6 @@ void CodeGen::genConsumeRegs(GenTree* tree) | |
| // Return Value: | ||
| // None. | ||
| // | ||
| // Notes: | ||
| // Note that this logic is localized here because we must do the liveness update in | ||
| // the correct execution order. This is important because we may have two operands | ||
| // that involve the same lclVar, and if one is marked "lastUse" we must handle it | ||
| // after the first. | ||
|
|
||
| void CodeGen::genConsumeOperands(GenTreeOp* tree) | ||
| { | ||
|
|
@@ -1389,6 +1399,55 @@ void CodeGen::genConsumeOperands(GenTreeOp* tree) | |
| } | ||
| } | ||
|
|
||
| #ifdef FEATURE_HW_INTRINSICS | ||
| //------------------------------------------------------------------------ | ||
| // genConsumeHWIntrinsicOperands: Do liveness update for the operands of a GT_HWIntrinsic node | ||
| // | ||
| // Arguments: | ||
| // node - the GenTreeHWIntrinsic node whose operands will have their liveness updated. | ||
| // | ||
| // Return Value: | ||
| // None. | ||
| // | ||
|
|
||
| void CodeGen::genConsumeHWIntrinsicOperands(GenTreeHWIntrinsic* node) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be nice to centralize the asserting of
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. |
||
| { | ||
| int numArgs = HWIntrinsicInfo::lookupNumArgs(node); | ||
| GenTree* op1 = node->gtGetOp1(); | ||
| if (op1 == nullptr) | ||
| { | ||
| assert((numArgs == 0) && (node->gtGetOp2() == nullptr)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: It is generally nice to break
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I generally disagree. Asserts already clutter the code, and though they are useful, they should impact the readability & flow of the code as little as possible, IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this is another bit of code that will go away when I get rid of |
||
| return; | ||
| } | ||
| if (op1->OperIs(GT_LIST)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we have a convention for this, but the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't bother too much with this. Hopefully we'll just get rid of |
||
| { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. |
||
| int foundArgs = 0; | ||
| assert(node->gtGetOp2() == nullptr); | ||
| for (GenTreeArgList* list = op1->AsArgList(); list != nullptr; list = list->Rest()) | ||
| { | ||
| GenTree* operand = list->Current(); | ||
| genConsumeRegs(operand); | ||
| foundArgs++; | ||
| } | ||
| assert(foundArgs == numArgs); | ||
tannergooding marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| else | ||
| { | ||
| genConsumeRegs(op1); | ||
| GenTree* op2 = node->gtGetOp2(); | ||
| if (op2 != nullptr) | ||
| { | ||
| genConsumeRegs(op2); | ||
| assert(numArgs == 2); | ||
| } | ||
| else | ||
| { | ||
| assert(numArgs == 1); | ||
| } | ||
| } | ||
| } | ||
| #endif // FEATURE_HW_INTRINSICS | ||
|
|
||
| #if FEATURE_PUT_STRUCT_ARG_STK | ||
| //------------------------------------------------------------------------ | ||
| // genConsumePutStructArgStk: Do liveness update for the operands of a PutArgStk node. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GT_HWIntrinsicis already handled a few lines below.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I missed that. I don't believe that we have (or will have) any Arm64 intrinsics that can be contained, so I'll remove the one further down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did this get addressed? I don't see the matching removal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit hard to see, with the way the diffs are shown, but if you look below, this line was deleted:
and the
genConsumeRegbelow it is actually the one for theOperIsInitValcase.