[PATCH] D23559: [ADCE] Add control dependence computation

[David Callahan via llvm-commits]

david2050 added inline comments.

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:459
@@ -395,2 +458,3 @@
   void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.setPreservesCFG();
+    AU.addRequired<PostDominatorTreeWrapperPass>();
+    AU.setPreservesCFG(); // TODO -- will remove when we start removing branches
----------------
mehdi_amini wrote:
> david2050 wrote:
> > mehdi_amini wrote:
> > > How expensive is this?
> > In email, Daniel Berlin discusses this as well.
> > 
> > I measured ~400 cpp source files internally and impact is about ~0.6% (that is end to end starting with C++ source)
> Thanks!
> 
> 0.6% is not negligible: some LLVM users are starting from bitcode (i.e. they don't pay the frontend cost) so the % could be considerably higher, and have low-latency requirements.
> 
> It seems there is a tradeoff where the "existing" "fast" ADCE may still be desirable. Do you remember if this was addressed in a previous email thread? (I kind of remember there was a fairly extensive discussion on your first submissions)
The previous discussion focused on the negative maintenance costs of having two implementations. That said, it clearly could choose to not build the PDT and forego control flow removal at lower optimiation levels.  Then we would pay a smaller tax (~ double the number of hash table lookups) for the reduced functionality.


https://reviews.llvm.org/D23559