JIT: update debug dumping for loop hoisting #59287

AndyAyersMS · 2021-09-17T22:27:13Z

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.

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 · 2021-09-17T22:27:18Z

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 · 2021-09-17T22:31:11Z

@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

Thanks for doing this!

BruceForstall · 2021-09-17T22:42:51Z

src/coreclr/jit/optimizer.cpp

@@ -5688,6 +5691,7 @@ void Compiler::optHoistLoopCode()
    {
        if (optLoopTable[lnum].lpFlags & LPFLG_REMOVED)
        {
+            JITDUMP("\nLoop L%02u was removed\n", lnum);


We now have FMT_LP

BruceForstall · 2021-09-17T22:43:40Z

src/coreclr/jit/optimizer.cpp

@@ -5746,7 +5750,7 @@ void Compiler::optHoistLoopCode()
 void Compiler::optHoistLoopNest(unsigned lnum, LoopHoistContext* hoistCtxt)
 {
    // 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);


BruceForstall · 2021-09-17T22:45:24Z

src/coreclr/jit/optimizer.cpp

    ArrayStack<BasicBlock*> defExec(getAllocatorLoopHoist());
    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);


typo: hosting

BruceForstall · 2021-09-17T22:45:33Z

src/coreclr/jit/optimizer.cpp

@@ -5947,12 +5970,16 @@ void Compiler::optHoistThisLoop(unsigned lnum, LoopHoistContext* hoistCtxt)
        // 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 "


typo: hosting

BruceForstall · 2021-09-17T22:45:43Z

src/coreclr/jit/optimizer.cpp

            defExec.Reset();
        }
        defExec.Push(pLoopDsc->lpEntry);
    }
    else // More than one exit
    {
+        JITDUMP("  only considering hosting in entry block " FMT_BB "\n", pLoopDsc->lpEntry->bbNum);


typo: hosting

BruceForstall · 2021-09-17T22:46:58Z

src/coreclr/jit/optimizer.cpp

@@ -6032,6 +6059,8 @@ bool Compiler::optIsProfitableToHoistableTree(GenTree* tree, unsigned lnum)
        // 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,


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

BruceForstall · 2021-09-17T22:48:47Z

src/coreclr/jit/optimizer.cpp

@@ -6415,6 +6475,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack<BasicBlock*>* blo
                if (!child.m_invariant)
                {
                    treeIsInvariant = false;
+                    INDEBUG(failReason = "variant child";);


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

EgorBo · 2021-09-19T16:06:30Z

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

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.

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 17, 2021

BruceForstall requested changes Sep 17, 2021

View reviewed changes

feedback

00e55b9

BruceForstall approved these changes Sep 18, 2021

View reviewed changes

AndyAyersMS merged commit eafc6f1 into dotnet:main Sep 18, 2021

AndyAyersMS deleted the WhyNotHoist2 branch September 18, 2021 01:42

ghost locked as resolved and limited conversation to collaborators Nov 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT: update debug dumping for loop hoisting #59287

JIT: update debug dumping for loop hoisting #59287

AndyAyersMS commented Sep 17, 2021

ghost commented Sep 17, 2021

AndyAyersMS commented Sep 17, 2021

BruceForstall left a comment

BruceForstall Sep 17, 2021

BruceForstall Sep 17, 2021

BruceForstall Sep 17, 2021

BruceForstall Sep 17, 2021

BruceForstall Sep 17, 2021

BruceForstall Sep 17, 2021

BruceForstall Sep 17, 2021

EgorBo commented Sep 19, 2021

JIT: update debug dumping for loop hoisting #59287

JIT: update debug dumping for loop hoisting #59287

Conversation

AndyAyersMS commented Sep 17, 2021

ghost commented Sep 17, 2021

AndyAyersMS commented Sep 17, 2021

BruceForstall left a comment

Choose a reason for hiding this comment

BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

BruceForstall Sep 17, 2021

Choose a reason for hiding this comment

EgorBo commented Sep 19, 2021