[PATCH] D36010: [Polly][Simplify] Implement write accesses coalescing.

[Tobias Grosser via Phabricator via llvm-commits]

grosser accepted this revision.
grosser added a comment.
This revision is now accepted and ready to land.

Very nice!



================
Comment at: lib/Transform/Simplify.cpp:260
+
+      // We let isl do the lookup for the same the value condition. For this, we
+      // wrap llvm::Value into an isl::set such that isl can do the lookup in
----------------
the same the value ???


================
Comment at: lib/Transform/Simplify.cpp:290
+        // In region statements, the explicit accesses can be in blocks that are
+        // can be executed in any order. We therefore process only the implicit
+        // writes and stop after that.
----------------
are can be


================
Comment at: lib/Transform/Simplify.cpp:308
+          // element is written, but it can return nullptr; For PHI accesses,
+          // getAccessValue() returns the PHI instead the PHI's incoming value.
+          // In this case where we are only compare values of a single
----------------
instead OF


================
Comment at: lib/Transform/Simplify.cpp:309
+          // getAccessValue() returns the PHI instead the PHI's incoming value.
+          // In this case where we are only compare values of a single
+          // statement, this is fine, because within a statement, a PHI in a
----------------
In THE case where we ARE??? only compare


================
Comment at: lib/Transform/Simplify.cpp:311
+          // statement, this is fine, because within a statement, a PHI in a
+          // successor block is always the same value as the incoming write. We
+          // still preferably use the incoming value directly so we also catch
----------------
HAS always


================
Comment at: lib/Transform/Simplify.cpp:333
+
+          // We are looking for a compatible other write accesses. That other
+          // write can access these elements...
----------------
for A??? compatible other write ACCESS??

Singular or plural. Please also check together with the following sentence.


================
Comment at: lib/Transform/Simplify.cpp:384
+        // the written elements) between them. Remove all visited write accesses
+        // from the list of eligible writes. Don't just remove the accesses
+        // elements, but any MemoryAccess that touches any of the invalidated
----------------
accessed


https://reviews.llvm.org/D36010