[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:143
+  llvm_unreachable("EndInst doesn't post dominate StartInst?");
+}
----------------
bmahjour wrote:
> 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. 
It's not as bad fortunately. A segfault (or similar) caused by a memory access is undefined behavior (UB).

What we need to make sure is that we do not move a store (or similar) across something that:
  might synchronize (but nothing else, incl. readnone)
  might throw       (same as above)
  might never return = endless loop (only not move it before and only if the pointer is not known dereferenceable and never return call/loop is side effect and synchronization free).

We probably want tests for these.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D70049