All tests under Transforms/PruneEH/ now pass under NPM.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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.
llvm/lib/Transforms/IPO/PruneEH.cpp | ||
---|---|---|
75–76 | 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. 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. } | |
85–86 | This check for null won't be needed then. | |
86–87 | The early exit condition !SCCMightUnwind || !SCCMightReturn could be triggered here, so perhaps add: if (SCCMightUnwind && SCCMightReturn) break; |
CallGraphUpdater: https://reviews.llvm.org/D87632
There I also added asserts that all Functions were not null.
asbirlea, you mentioned offline that there were passes that replaced this in the NPM and that this shouldn't be ported, do you know what those are so I can put them in the description of another change pinning tests to the legacy PM?
I think for prune-eh, the NPM inliner was intended to address the same cleanup, but I don't have all the context.
@chandlerc for more details.
Looking at the pass, it looks like it can be run independently of an inliner since it simply changes invokes into calls whenever possible.
There are lots of random tests that use this pass, and there's even a C API for it.
I think we should port this, and this patch is pretty small. Of course we can always remove it later if we decide to.
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.