[PATCH] D35264: [LICM] Teach LICM to hoist conditional loads

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:921
+      // query in favour of a densemap lookup.
+      if (CurLoop->contains(LSI->getParent()) ||
+          !DT->dominates(LSI->getParent(), PH))
----------------
The DT queries are O(1), so i'm not sure what this buys you?


================
Comment at: lib/Transforms/Scalar/LICM.cpp:949
   if (!GuaranteedToExecute) {
+    // Conditional loads may be hoisted if the address has already been accessed
+    // by a dominating block.
----------------
Domination does not guarantee execution.
It only guarantees that all paths that execute go through a given block before getting to another:

```
A
| \
B |
| /  
C

```
A dominates B and C, but only one of them will execute.

You want control dependence, and in particular, CDEQ sets, which divides blocks into equivalence classes where everything in the same equivalence class executes under the same set of conditions.

But you will find this is never going to be true for normal loops.



Repository:
  rL LLVM

https://reviews.llvm.org/D35264