Skip to content

JIT: update debug dumping for loop hoisting #59287

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
AndyAyersMS merged 2 commits into from
Sep 18, 2021
Merged

JIT: update debug dumping for loop hoisting #59287

AndyAyersMS merged 2 commits into from
Sep 18, 2021

Conversation

@AndyAyersMS
Copy link
Member

@AndyAyersMS AndyAyersMS commented Sep 17, 2021

I kept adding stuff like this whenever I wanted to understand why hosting was
doing what it was doing, so figured it likely had lasting value.

@AndyAyersMS
JIT: update debug dumping for loop hoisting
3db8885 
I kept adding stuff like this whenever I wanted to understand why hosting was
doing what it was doing, so figured it likely had lasting value.
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 17, 2021
@ghost
Copy link

ghost commented Sep 17, 2021

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

I kept adding stuff like this whenever I wanted to understand why hosting was
doing what it was doing, so figured it likely had lasting value.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr
Milestone: -

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Sep 17, 2021

@BruceForstall PTAL
cc @dotnet/jit-contrib

Sample jit dump fragment

BB02 [0001]  2       BB01,BB04             4     0 [006..00A)-> BB04 ( cond )                     i Loop Loop0 bwd bwd-target 
BB03 [0002]  2       BB02,BB03            16     1 [00A..01B)-> BB03 ( cond )                     i Loop Loop0 bwd bwd-target align 
BB04 [0004]  2       BB02,BB03             4     0 [01B..024)-> BB02 ( cond )                     i bwd 

Loop Nest L00
optHoistLoopCode for loop L00 <BB02..BB04>:
  Loop body does not contain a call
  Loop has single exit

  USEDEF  (5)={V03 V00 V01 V04 V02}
  INOUT   (4)={V03 V00 V01 V02}
  LOOPVARS(4)={V03 V00 V01 V02}
  Only considering hosting in blocks that dominate exit block BB04

    optHoistLoopBlocks BB02 (weight=  4   ) of loop L00 <BB02..BB04>, firstBlock is true
      [000012] not hoistable: variant child
      [000043] not hoistable: variant child

    optHoistLoopBlocks BB04 (weight=  4   ) of loop L00 <BB02..BB04>, firstBlock is false
      [000036] not hoistable: variant child
      [000009] not hoistable: variant child

Note we skip hosting from the inner loop block BB03 because it is conditionally executed. Added todos to the code for this (echoing some of what's over in #35735).

BruceForstall
BruceForstall suggested changes Sep 17, 2021
View reviewed changes
Copy link
Contributor

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this!

src/coreclr/jit/optimizer.cpp Outdated
{
if (optLoopTable[lnum].lpFlags & LPFLG_REMOVED)
{
JITDUMP("\nLoop L%02u was removed\n", lnum);
Copy link
Contributor

@BruceForstall BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now have FMT_LP

src/coreclr/jit/optimizer.cpp Outdated
{
// Do this loop, then recursively do all nested loops.
CLANG_FORMAT_COMMENT_ANCHOR;
JITDUMP("\n%s L%02u\n", optLoopTable[lnum].lpParent == BasicBlock::NOT_IN_LOOP ? "Loop Nest" : "Nested Loop", lnum);
Copy link
Contributor

@BruceForstall BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

src/coreclr/jit/optimizer.cpp Outdated
if (pLoopDsc->lpFlags & LPFLG_ONE_EXIT)
{
assert(pLoopDsc->lpExit != nullptr);
JITDUMP(" Only considering hosting in blocks that dominate exit block " FMT_BB "\n", pLoopDsc->lpExit->bbNum);
Copy link
Contributor

@BruceForstall BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: hosting

src/coreclr/jit/optimizer.cpp Outdated
// If we didn't reach the entry block, give up and *just* push the entry block.
if (cur != pLoopDsc->lpEntry)
{
JITDUMP(" -- odd, we didn't reach entry from exit via dominators. Only considering hosting in entry "
Copy link
Contributor

@BruceForstall BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: hosting

src/coreclr/jit/optimizer.cpp Outdated
}
else // More than one exit
{
JITDUMP(" only considering hosting in entry block " FMT_BB "\n", pLoopDsc->lpEntry->bbNum);
Copy link
Contributor

@BruceForstall BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: hosting

src/coreclr/jit/optimizer.cpp Outdated
// Don't hoist expressions that are not heavy: tree->GetCostEx() < (2*IND_COST_EX)
if (tree->GetCostEx() < (2 * IND_COST_EX))
{
JITDUMP(" tree cost too low: %d < %d (lvc %u >= arc %u)\n", tree->GetCostEx(), 2 * IND_COST_EX,
Copy link
Contributor

@BruceForstall BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, spelling out the variables here would be better. (same below)

src/coreclr/jit/optimizer.cpp Outdated
if (!child.m_invariant)
{
treeIsInvariant = false;
INDEBUG(failReason = "variant child";);
Copy link
Contributor

@BruceForstall BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I presume you only need one of the semicolons (here and below)

@AndyAyersMS AndyAyersMS merged commit eafc6f1 into dotnet:main Sep 18, 2021
@AndyAyersMS AndyAyersMS deleted the WhyNotHoist2 branch September 18, 2021 01:42
@EgorBo
Copy link
Member

EgorBo commented Sep 19, 2021

Oh, I was going to use "Loop Hoisting" as an example of a jit area that needs more diagnostic info in my write-up 😐

@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@BruceForstall BruceForstall BruceForstall approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS @EgorBo @BruceForstall