[PATCH] D120096: PGOInstrumentation, GCOVProfiling: Split indirectbr critical edges regardless of PHIs

[Di Mo via Phabricator via llvm-commits]

modimo accepted this revision.
modimo added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/test/Transforms/GCOVProfiling/split-indirectbr-critical-edges.ll:37
+indirect2:
+  ; A 2nd branch to %indirect should make it impossibel to split the edge.
+  indirectbr i8* %2, [label %indirect, label %end]
----------------
MatzeB wrote:
> modimo wrote:
> > nit:s/impossibel/impossible/g
> > 
> > nit2: If we're willing to change the labels the indirect gotos jump to we can technically split an arbitrary # of edges. Not that we should, of course..
> Not completely sure what you mean in "nit2", but I changed the wording to make it clear that this is a trick to keep the edge from getting split so the test keeps working (I know more normalization etc. could "fix" this, but we're only running a single pass here)
You got it, "nit2" was that handling this case isn't technically impossible. Thanks for the wording change.


================
Comment at: llvm/test/Transforms/PGOProfile/irreducible.ll:142
 ; USE: ![[SW_BB15_IRR_LOOP]] = !{!"loop_header_weight", i64 100}
-; USE: ![[INDIRECTGOTO_IRR_LOOP]] = !{!"loop_header_weight", i64 400}
+; USE: ![[INDIRECTGOTO_IRR_LOOP]] = !{!"loop_header_weight", i64 399}
----------------
MatzeB wrote:
> modimo wrote:
> > I'm curious how the counters shifted around now that indirectgoto->sw.bb gets split. How does the changed counter values in the profiles above map to that?
> This is hard to figure out given the size of the test and the fact that the profile files are checked-in as-is. I hope this is fine...
Took a look because I was curious how to get this information. Adding `-debug-only=pgo-instrumentation` dumps the edges and the instrumented edges line up with the counts in the profile. Comparing the two (irreducible.proftext) left is original right is with this diff:
{F22208556}

The count of `100` gets inferred now and the new counts are for the split edges as expected. The change that causes the `INDIRECTGOTO_IRR_LOOP` to go from 400->399 is that the probes shifted so that edge 21 and edge 22 on RHS flipped compared to LHS. That's not a big deal though.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120096/new/

https://reviews.llvm.org/D120096