[PATCH] D70049: [CodeMoverUtils] Added an API to check if an instruction can be safely moved before another instruction.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:84
+          return false;
+  }
+
----------------
Why PDT. domainates? Operands need to dominate the use:
`DT.dominates(OpInst, InsertPoint)`

Below, dominance is allowing the move but post-dominance would prevent it.
```
a = add ...
if (abc) {
   IP = ...
   [..]
   I = add a, ... 
}
```




================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:143
+  llvm_unreachable("EndInst doesn't post dominate StartInst?");
+}
----------------
High-level question:

Does dependence info track dependences between side-effects (e.g., global write) and potentially throwing instructions?
E.g., would this allow me to move `I` to `IP` in the example below:
```
IP = ...
call void @unknown() readnone
store ... // <- I
```
Thinking about it, if you move a (generic) side-effect you need to check `nounwind` and `willreturn` for all instructions you move it over.


Nits:
No need for `inline` (let the compiler figure it out).
If you reorder, forward declarations not necessary (= easier to maintain).




Repository:
  rL LLVM

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

https://reviews.llvm.org/D70049