[jakobbotsch]

It does not yet support intermixed EH regions and loops (which I think the old stuff does), but this just makes it so we at least can get something for the new loops.

Example

[ghost]

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

Issue Details

---

It does not yet support intermixed EH regions and loops (which I think the old stuff does), but this just makes it so we at least can get something for the new loops.

Author:	jakobbotsch
Assignees:	jakobbotsch
Labels:	area-CodeGen-coreclr
Milestone:	-

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @BruceForstall

[BruceForstall]

I think you shouldn't remove the old loop region graph dumping... yet. Maybe you can add a DOTNET config to dump the old one and make dumping the new one the default?

Why couldn't you use the existing RegionGraph graph structure and add the loop into it instead of writing your own dumper?

Presumably new loops are properly nested within EH regions, or properly encompass entire EH regions, so wouldn't inserting the new loops in the existing RegionGraph as is done with the old loops give support for displaying both?

I guess the issue for new loops is loop nesting is not lexical (by block ranges). Although there should still be block-disjoint well-formed loop nesting, and that nesting should be well-formed w.r.t. EH regions.

[jakobbotsch]

I think you shouldn't remove the old loop region graph dumping... yet. Maybe you can add a DOTNET config to dump the old one and make dumping the new one the default?

That's fair, I can do that.

Why couldn't you use the existing RegionGraph graph structure and add the loop into it instead of writing your own dumper?

Presumably new loops are properly nested within EH regions, or properly encompass entire EH regions, so wouldn't inserting the new loops in the existing RegionGraph as is done with the old loops give support for displaying both?

I guess the issue for new loops is loop nesting is not lexical (by block ranges).

Right, that's the problem.

Although there should still be block-disjoint well-formed loop nesting, and that nesting should be well-formed w.r.t. EH regions.

While there is some form of nesting property, it is not generally true that EH regions are well-scoped with regards to the parent loops. That is, not all blocks of an EH region inside a loop are necessarily part of the loop.
Consider the following example:

for (int i = 0; i < 100; i++)
{
  try
  {
    // BB01
    if (i == 50)
    {
      // BB02
      break;
    }
    // BB03
    Foo();
  }
  catch
  {
    // BB04
    break;
  }
}

The loop here contains BB01 and BB03, but it does not contain BB02 and BB04. On the other hand, the EH region does contain BB01, BB02 and BB03. So there is no general way to draw a nested relationship between the loop and (full) EH region, I think.

[jakobbotsch]

I've reintroduced the old loop dumping under JitDumpFgOldLoops=1, can you please take another look @BruceForstall?

[BruceForstall]

Regarding your example: note that an EH "region" is either the 'try' or the handler, not both: the 'try' and handler are considered separately. So in your example, the loop BB01/BB03 are properly nested within the 'try'.

The change is that the RegionGraph as currently implemented depends on BasicBlock bbNum ranges to determine nesting. For new loops, you would have to tell it both the set of blocks in a loop and what the nesting relationship is between a loop and it's parent loop. (Or have a separate implementation entirely, if warranted.)

But it's still valid, I think, to be able to represent EH and loop nesting.

[BruceForstall]

Thanks for adding this.

[jakobbotsch]

Regarding your example: note that an EH "region" is either the 'try' or the handler, not both: the 'try' and handler are considered separately. So in your example, the loop BB01/BB03 are properly nested within the 'try'.

The regions for the loop and try would intersect and be disjoint; the try does not contain all blocks of the loop (e.g. i++) and the loop does not contain all blocks of the try (e.g. BB02), but they do share several blocks (e.g. BB01). So there is no nesting relationship.

[BruceForstall]

ok, makes sense.