[PATCH] D87625: [PruneEH][NewPM] Port prune-eh to NPM

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

I think for clarify this could be split into a patch that updates the current pass to use the CallGraphUpdater, and a patch that only adds the pass for the new pass manager.



================
Comment at: llvm/lib/Transforms/IPO/PruneEH.cpp:72
   // according to what we know.
-  for (CallGraphNode *I : SCC)
-    if (Function *F = I->getFunction())
-      MadeChange |= SimplifyFunction(F, CG);
+  for (Function *F : Functions)
+    MadeChange |= SimplifyFunction(F, CGU);
----------------
I'm not clear if at this point we guarantee that there are no nullptrs in Functions. It would be good to pre-check this.
e.g.
```
if (Functions.contains (nullptr)) {
  SCCMightUnwind = true;
  SCCMightReturn = true;
  Functions.delete(nullptr); //then the for here and at line 158 doesn't need the check which was in the original and removed here.
}
```


================
Comment at: llvm/lib/Transforms/IPO/PruneEH.cpp:84
+  for (Function *F : Functions) {
     if (!F) {
       SCCMightUnwind = true;
----------------
This check for null won't be needed then.


================
Comment at: llvm/lib/Transforms/IPO/PruneEH.cpp:87
       SCCMightReturn = true;
     } else if (!F->hasExactDefinition()) {
       SCCMightUnwind |= !F->doesNotThrow();
----------------
The early exit condition `!SCCMightUnwind || !SCCMightReturn` could be triggered here, so perhaps add:
```
if (SCCMightUnwind && SCCMightReturn)
          break;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87625