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

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:143
+  llvm_unreachable("EndInst doesn't post dominate StartInst?");
+}
----------------
jdoerfert wrote:
> 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).
> 
> 
> Does dependence info track dependences between side-effects (e.g., global write) and potentially throwing instructions

Dependence analysis knows nothing about function calls, so it makes worst case assumption and assumes there is confused dependency between calls and other memory accesses (even if the call takes no arguments and is readnone). This should be safe for now but is obviously too pessimistic. I think dependence analysis could be improved to take some obvious cases of function calls into account, but I don't think exception handling would be something it can be taught about. 

It also raises the question, to what extend do we need to be worried about exceptions and side-effects at higher optimization levels? For instance one could go as far as saying any store cannot be moved past any other store (even if the memories accessed are disjoint) because for example the first store could cause a segfault before the second store has had a chance to execute. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D70049