Comments and cleanup for loop cloning #49768
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
@dotnet/jit-contrib |
BruceForstall
force-pushed
the
LoopOpts3
branch
from
88bbe7e to
1a9c7bb
Compare
AndyAyersMS
approved these changes
Mar 21, 2021
| @@ -346,6 +346,9 @@ void CodeGen::genCodeForBBlist() | |||
| if ((block->bbPrev != nullptr) && (block->bbPrev->bbJumpKind == BBJ_COND) && | |||
| (block->bbWeight != block->bbPrev->bbWeight)) | |||
| { | |||
| JITDUMP("Adding label due to BB weight difference: BBJ_COND " FMT_BB " with weight " FMT_WT | |||
There was a problem hiding this comment.
While you are here, I am wondering if there is an easy way to add towards the end the mapping of BB -> IG somehow? While looking at JITDump, it is not straightforward to find this mapping and is very helpful overall when we look backward in JITDump.
kunalspathak
approved these changes
Mar 22, 2021
| @@ -43,45 +117,78 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX | |||
|
|
|||
| Heuristic: | |||
| Therefore we will not clone if we exceed creating 4 blocks. | |||
| Note: this means we never clone more than 2-dimension a[i][j] expressions | |||
| (see optComputeDerefConditions()). | |||
| REVIEW: make this heuristic defined by a COMPlus variable, for easier | |||
There was a problem hiding this comment.
make this heuristic defined by a COMPlus variable,
+1 on that.
This is a no-diff change Added various comments to document loop cloning. Standardized and improved some logging. Consolidated more loop cloning condition checking into `optIsLoopClonable` that was previously in `optIdentifyLoopOptInfo`. Replaced some `0` weights with `BB_ZERO_WEIGHT`. Made FMT_BB use pervasive.
Added FMT_LP formatting string. Cached often-used `optLoopTable[loopInd]` expression. Added `const` to many loop query member functions. Added static `GenTree::OperIs(compareOper, oper, oper, oper...)` functions for simplifying oper check expressions where the compareOper isn't from a GenTree node `OperGet()`. (This doesn't need to be part of GenTree, but it doesn't hurt, either.) Added a few more comments.
BruceForstall
force-pushed
the
LoopOpts3
branch
from
1a9c7bb to
3653f16
Compare
Rename to StaticOperIs. It appears the compiler uses the wrong template in some cases, but doesn't complain about duplicate options, leading to run-time failures.
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a no-diff change
Added various comments to document loop cloning.
Standardized and improved some logging.
Consolidated more loop cloning condition checking into
optIsLoopClonablethat was previously inoptIdentifyLoopOptInfo.Replaced some
0weights withBB_ZERO_WEIGHT.Made FMT_BB use pervasive.