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

[Di Mo via Phabricator via llvm-commits]

modimo added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h:503
 // If BPI and BFI aren't non-null, BPI/BFI will be updated accordingly.
-bool SplitIndirectBrCriticalEdges(Function &F,
+bool SplitIndirectBrCriticalEdges(Function &F, bool IgnoreBlocksWithoutPHI,
                                   BranchProbabilityInfo *BPI = nullptr,
----------------
Please document this flag in the comment above


================
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]
----------------
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..


================
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}
----------------
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?


================
Comment at: llvm/unittests/Transforms/Utils/BasicBlockUtilsTest.cpp:475
+
+  // bb3 has no PHI, so we shouldn't split bb0 -> bb2
+  BasicBlock *BB0 = getBasicBlockByName(*F, "bb0");
----------------
`s/bb3/bb2/g` has no PHI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120096