[PATCH] D29865: [PDSE] Add a no-op pass.

[bryant via Phabricator via llvm-commits]

bryant marked 3 inline comments as done.
bryant added inline comments.


================
Comment at: lib/Transforms/Scalar/PDSE.cpp:59
+// (resp. post-dominate) their successor (resp. predecessor) blocks (SSAPRE
+// assumes this condition).
+
----------------
> This TODO really belongs here or you can throw it away?

SSAPRE (resp. PDSE) needs critical edges to be broken in order for down-unsafety (resp. up-unsafety) propagation to be correct. I'm not sure if this condition is already met by default. I've tried adding BreakCriticalEdges as dependency, but this causes a crash in the legacy pass manager:

```
diff --git a/lib/Transforms/Scalar/PDSE.cpp b/lib/Transforms/Scalar/PDSE.cpp
index 941b184..f576260 100644
--- a/lib/Transforms/Scalar/PDSE.cpp
+++ b/lib/Transforms/Scalar/PDSE.cpp
@@ -723,11 +723,13 @@ public:
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<PostDominatorTreeWrapperPass>();
     AU.addRequired<AAResultsWrapperPass>();
+    AU.addRequiredID(BreakCriticalEdgesID);
     AU.addRequired<TargetLibraryInfoWrapperPass>();
 
     AU.setPreservesCFG();
     AU.addPreserved<PostDominatorTreeWrapperPass>();
     AU.addPreserved<GlobalsAAWrapperPass>();
+    AU.addPreservedID(BreakCriticalEdgesID);
   }
 
   static char ID; // Pass identification, replacement for typeid
@@ -741,6 +743,7 @@ INITIALIZE_PASS_BEGIN(PDSELegacyPass, "pdse", "Partial Dead Store Elimination",
 INITIALIZE_PASS_DEPENDENCY(PostDominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(BreakCriticalEdges)
 INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
 INITIALIZE_PASS_END(PDSELegacyPass, "pdse", "Partial Dead Store Elimination",
                     false, false)
```

```
Args: opt -debug -debug-pass=Details -print-before-all -print-after-all -print-frg -pdse test.ll 
 -- 'Break critical edges in CFG' is not preserving 'Function Alias Analysis Results'
 -- 'Break critical edges in CFG' is not preserving 'Print function to stderr'
 -- 'Break critical edges in CFG' is not preserving 'Post-Dominator Tree Construction'
 -- 'Break critical edges in CFG' is not preserving 'Basic Alias Analysis (stateless AA impl)'
 -- 'Break critical edges in CFG' is not preserving 'Function Pass Manager'
Pass Arguments:  -targetlibinfo -tti -assumption-cache-tracker -postdomtree -domtree -basicaa -aa -print-function -break-crit-edges -print-function -print-function
Target Library Information
Target Transform Information
Assumption Cache Tracker
  ModulePass Manager
    FunctionPass Manager
      Post-Dominator Tree Construction
      Dominator Tree Construction
      Basic Alias Analysis (stateless AA impl)
      Function Alias Analysis Results
      Print function to stderr
      Break critical edges in CFG
      Print function to stderr
      Print function to stderr
Unable to schedule 'Post-Dominator Tree Construction' required by 'Partial Dead Store Elimination'
Unable to schedule pass
UNREACHABLE executed at /home/bryant/3rd/llvm/lib/IR/LegacyPassManager.cpp:1243!
#0 0x00007ff310f4d989 llvm::sys::PrintStackTrace(llvm::raw_ostream&) /home/bryant/3rd/llvm/build/../lib/Support/Unix/Signals.inc:402:11
#1 0x00007ff310f4dba9 PrintStackTraceSignalHandler(void*) /home/bryant/3rd/llvm/build/../lib/Support/Unix/Signals.inc:466:1
#2 0x00007ff310f4aaa7 llvm::sys::RunSignalHandlers() /home/bryant/3rd/llvm/lib/Support/Signals.cpp:0:5
#3 0x00007ff310f4df30 SignalHandler(int) /home/bryant/3rd/llvm/build/../lib/Support/Unix/Signals.inc:256:1
#4 0x00007ff3102988d0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0xf8d0)
#5 0x00007ff30f8ea067 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x35067)
#6 0x00007ff30f8eb448 abort (/lib/x86_64-linux-gnu/libc.so.6+0x36448)
#7 0x00007ff310e264a0 LLVMInstallFatalErrorHandler /home/bryant/3rd/llvm/lib/Support/ErrorHandling.cpp:133:0
#8 0x00007ff312250664 /home/bryant/3rd/llvm/lib/IR/LegacyPassManager.cpp:1243:3
#9 0x00007ff31224f288 llvm::PMDataManager::add(llvm::Pass*, bool) /home/bryant/3rd/llvm/lib/IR/LegacyPassManager.cpp:1018:22
#10 0x00007ff31225374a llvm::FunctionPass::assignPassManager(llvm::PMStack&, llvm::PassManagerType) /home/bryant/3rd/llvm/lib/IR/LegacyPassManager.cpp:1858:1
#11 0x00007ff31224c4b5 llvm::PMTopLevelManager::schedulePass(llvm::Pass*) /home/bryant/3rd/llvm/lib/IR/LegacyPassManager.cpp:690:7
#12 0x00007ff31225e12f llvm::legacy::PassManagerImpl::add(llvm::Pass*) /home/bryant/3rd/llvm/lib/IR/LegacyPassManager.cpp:398:3
#13 0x00007ff312252e11 llvm::legacy::PassManager::add(llvm::Pass*) /home/bryant/3rd/llvm/lib/IR/LegacyPassManager.cpp:1719:1
#14 0x0000000000270a3f (opt+0x270a3f)
#15 0x000000000026c2f8 (opt+0x26c2f8)
#16 0x00007ff30f8d6b45 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b45)
#17 0x0000000000242029 (opt+0x242029)
Stack dump:
0.      Program arguments: opt -debug -debug-pass=Details -print-before-all -print-after-all -print-frg -pdse test.ll 
```

If need be, I could fix this problem first before committing the patch.



Repository:
  rL LLVM

https://reviews.llvm.org/D29865