[PATCH] D142747: [Pseudo Probe] Do not instrument EH blocks.

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

In D142747#4086978 <https://reviews.llvm.org/D142747#4086978>, @modimo wrote:

> We've got the same EH-ness propagation code in MFS already: https://reviews.llvm.org/D131824. Consider merging the code?

That diff also used EHPad as seeds for BFS, I thought we only need LandingPads as seeds? Though I think for simple algorithm, having MIR and IR implementation separated is probably fine. I also don't know where to put a merged version..

  if (MBB.isEHPad())
    LandingPads.push_back(&MBB);



================
Comment at: llvm/lib/Transforms/IPO/SampleProfileProbe.cpp:278-279
+      for (auto *Predecessor : predecessors(Successor)) {
+        if (Predecessor == Successor)
+          continue;
+        if (!EHBlocks.contains(Predecessor)) {
----------------
hoy wrote:
> wenlei wrote:
> > hoy wrote:
> > > wenlei wrote:
> > > > Is this necessary? If Predecessor is non-EH, we will break out the loop later; if it's EH, line 274 should have skipped this Predecessor/Successor already. 
> > > It's necessary if the successor forms a self-loop. In such case, the self back edge should be ignored.
> > What I'm saying is the self back edge will be ignored without this check (see reasoning above), or am I wrong? 
> Without this check the self back edge is going to be ignored in a different way such that the block will never be considered EH-only.
Ok, but if the purpose is to prevent predecessor from cycle to be considered non-EH. How about landingpad->A->B->A, where there's no self edge to A, but B would prevent A from being marked EH? The need for self cycle special handling looks fishy.. We either handle all cycles, or don't.. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142747