[PATCH] D80400: [DwarfEHPrepare] Don't prune unreachable resumes at optnone

[Reid "Away June-Sep" Kleckner via Phabricator via llvm-commits]

rnk added a subscriber: compnerd.
rnk added a comment.

Neat



================
Comment at: include/llvm/CodeGen/Passes.h:343
   /// adapted to code generation.  Required if using dwarf exception handling.
-  FunctionPass *createDwarfEHPass();
+  FunctionPass *createDwarfEHPass(bool PruneUnreachableResumes);
 
----------------
IMO it makes more sense to pass in the optimization level, and then let the pass decide what that means it should do, so the caller doesn't have to think as much about the meaning of this parameter. It will simplify the call site as well.


================
Comment at: lib/CodeGen/DwarfEHPrepare.cpp:150
     SmallVectorImpl<LandingPadInst *> &CleanupLPads) {
+  if (!PruneUnreachableResumes)
+    return Resumes.size();
----------------
I think it makes more sense to check this at the caller side.


================
Comment at: test/CodeGen/X86/O0-pipeline.ll:30
 ; CHECK-NEXT:       Expand indirectbr instructions
 ; CHECK-NEXT:     Rewrite Symbols
 ; CHECK-NEXT:     FunctionPass Manager
----------------
As a separate issue, this module pass between all the function passes is probably bad for cache locality. My understanding is that it acts as a barrier. We'll get these loops:
  for all functions:
    for some passes:
      run fn passes
  run rewrite symbol pass
  for all functions
    for remaining passes:
      run fn pass

Instead of this loop:
  for all functions:
    for all fn passes:
      run fn pass

I bet we can make createRewriteSymbolsPass return null if the YAML file option is not set, and then remove this from the standard pipeline.

@compnerd


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80400