[PATCH] D37298: [ForwardOpTree] Allow forwarding in the presence of region statements

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Your test cases check that the movement of non-load scalars to/from region works now (Which is amazing!).  A test that checks that a load is not forwarded in case there is an (unpredictable) write in a region is missing. It might load the content written the store instead of the original value.

Simplify.cpp markAndSweep needs to know about the prepended instructions. Otherwise it doesn't see their operands and doesn't mark them as used, hence remove them.



================
Comment at: include/polly/ScopInfo.h:2369-2370
 
+  /// Return the LoopInfo used for this Scop.
+  LoopInfo *getLI() const { return Affinator.getLI(); }
+
----------------
Why this move?


================
Comment at: test/ForwardOpTree/forward_into_region.ll:51-57
+; CHECK-NEXT:     Instructions copied: 1
+; CHECK-NEXT:     Known loads forwarded: 0
+; CHECK-NEXT:     Read-only accesses copied: 0
+; CHECK-NEXT:     Operand trees forwarded: 1
+; CHECK-NEXT:     Statements with forwarded operand trees: 1
+; CHECK-NEXT: }
+; CHECK-NEXT: After statements {
----------------
Can you use `CHECK:` here? We might want to add additional stats which would cause this to break unnecessarily.


================
Comment at: test/ForwardOpTree/noforward_from_region.ll:32-35
+      %x = load double, double* %A
+      store double %x, double* %A
+      %y = load double, double* %B
+      store double %y, double* %B
----------------
These don't seem to be relevant.


https://reviews.llvm.org/D37298