[PATCH] Extend EarlyCSE to handle basic cases from JumpThreading and CVP

Daniel Berlin dberlin at dberlin.org
Mon May 18 17:35:53 PDT 2015


================
Comment at: include/llvm/Transforms/Utils/Local.h:290
@@ -288,1 +289,3 @@
 
+/// Replace each use of 'From' with 'To' if that use is dominated by the given
+/// edge.  Returns the number of replacements made.
----------------
\brief?

================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:471
@@ +470,3 @@
+  if (BasicBlock *Pred = BB->getSinglePredecessor())
+    // TODO: handle switch with case value as well
+    if (auto *BI = dyn_cast<BranchInst>(Pred->getTerminator()))
----------------
Even GVN does not handle switch with case value, even when it can determine the case value as a constant. It is not as simple as one would thing because it's not about dominance, but control dependence.


(and DT->dominates will fail the single edge assertion in those cases, even though it's a reasonable thing to test)


================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:487
@@ +486,3 @@
+                                     BasicBlockEdge(Pred, BB));
+            // TODO: Exploit icmp eq %v, C to get value for C
+            // TODO: Look through and/or sequences to get more known conditions
----------------
I'm not sure i agree with either of these TODO's.
It's really 100% unclear to me how far this rabbithole we should go for EarlyCSE 
I would just leave them (and the above one) out for now, and we can evaluate it when someone has data these are worthwhile to chase.

================
Comment at: test/Transforms/EarlyCSE/conditional.ll:1
@@ +1,2 @@
+; RUN: opt -early-cse -S < %s | FileCheck %s
+
----------------
Can you add descriptions of what each test is testing?
I can kind of figure it out, but not entirely.

Can you also add a few fallthrough edge tests similar to edge.ll from transforms/GVN to make sure it does the right thing?

In fact, i would just copy that and shove the right answers in for earlycse's current impl.

That will make sure nobody accidentally breaks the trickier cases here.

http://reviews.llvm.org/D9763

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list