[PATCH] D28147: [LICM] Allow promotion of some stores that are not guaranteed to execute

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

I'm responding to the description.  If the implementation splits a hair the description doesn't, my response may be off base.

I don't think this transform is legal.  The exception being thrown might be what prevents the otherwise faulting load or store from executing.

Consider this small example:
for(.i = ...) {

  throw_if_out_of_bounds(i);
  a[i] += 5;

}

We can't speculatively execute the load from a[i] as it might be the case that it crosses a page boundary and dereferencing the memory will introduce a page fault which doesn't exist in the original program.

(I'm deliberately using a loop variant i in the example, but the same thing applies if i is loop invariant.)



================
Comment at: lib/Transforms/Scalar/LICM.cpp:910
   // It is safe to promote P if all uses are direct load/stores and if at
-  // least one is guaranteed to be executed.
-  bool GuaranteedToExecute = false;
+  // least one store is guaranteed to be executed.
+  bool PromotionIsLegal = false;
----------------
You don't need a store.  A load is sufficient for this part.


https://reviews.llvm.org/D28147