- Notifications
You must be signed in to change notification settings - Fork5.1k
Fix perf issues discovered in "For software performance, can you always trust inlining" blog#61408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from1 commit
a433102167324472fb64edddf3494f4c70980897b43368ff2bd387828967fceFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
…n no-TC mode
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -893,7 +893,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed | ||
| unsigned retBlocks = 0; | ||
| int prefixFlags = 0; | ||
| bool preciseScan = makeInlineObservations && compInlineResult->GetPolicy()->RequiresPreciseScan(); | ||
| const bool resolveTokens = preciseScan; | ||
EgorBo marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| // Track offsets where IL instructions begin in DEBUG builds. Used to | ||
| // validate debug info generated by the JIT. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -3948,10 +3948,12 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) | ||
| costSz = 2 * 2; | ||
| break; | ||
| #if defined(FEATURE_HW_INTRINSICS) | ||
| case GT_HWINTRINSIC: | ||
| { | ||
| GenTreeHWIntrinsic* hwTree = tree->AsHWIntrinsic(); | ||
| #if defined(TARGET_XARCH) | ||
| if (hwTree->OperIsMemoryLoadOrStore()) | ||
| { | ||
| costEx = IND_COST_EX; | ||
| costSz = 2; | ||
| @@ -3964,9 +3966,32 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) | ||
| goto DONE; | ||
| } | ||
| } | ||
| #endif | ||
| switch (hwTree->gtHWIntrinsicId) | ||
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. Do we need to review costing for HW intrinsics more broadly? 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. We likely should. We probably aren't accounting for cases where helper intrinsics are more expensive than they appear nor cases where operands have less cost due to special handling that hardware intrinsics get. There's also probably cases where operands (like scalar DBL_CNS) are currently participating in overall CSE and shouldn't for certain cases. | ||
| { | ||
| #if defined(TARGET_XARCH) | ||
| case NI_Vector128_Create: | ||
| case NI_Vector256_Create: | ||
| #elif defined(TARGET_ARM64) | ||
| case NI_Vector64_Create: | ||
| case NI_Vector128_Create: | ||
| #endif | ||
| { | ||
| if (hwTree->gtGetOp1()->OperIsConst() && (hwTree->gtGetOp2() == nullptr)) | ||
| { | ||
| // Vector.Create(cns) is cheap but not that cheap to be (1,1) | ||
| costEx = 2; | ||
| costSz = 2; | ||
| } | ||
| break; | ||
| } | ||
| default: | ||
| break; | ||
| } | ||
| } | ||
| break; | ||
| #endif // FEATURE_HW_INTRINSICS | ||
| case GT_BLK: | ||
| case GT_IND: | ||