[PATCH] D19984: [LV] Preserve order of dependences in interleaved accesses analysis

[Matthew Simpson via llvm-commits]

Adam,

Thanks for the comments. I replied to you in Phabricator, but it doesn't look like my reply made it to the list for some reason.

http://reviews.llvm.org/D19984

-- Matt

-----Original Message-----
From: Adam Nemet [mailto:anemet at apple.com] 
Sent: Thursday, May 19, 2016 4:55 PM
To: mssimpso at codeaurora.org; hfinkel at anl.gov; silviu.baranga at arm.com; anemet at apple.com
Cc: mzolotukhin at apple.com; llvm-commits at lists.llvm.org; mcrosier at codeaurora.org
Subject: Re: [PATCH] D19984: [LV] Preserve order of dependences in interleaved accesses analysis

anemet added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5162-5166
@@ -5109,1 +5161,7 @@
 
+      // If instructions A and B cannot be reordered because they're in a
+      // dependence relation, we can't add additional instructions to A's group
+      // since this may cause illegal code motion. If B is already in a group
+      // release it. We check that B is a write because our code motion won't
+      // ever violate WAR dependences.
+      if (B->mayWriteToMemory() && !canReorderMemAccesses(B, A)) {
----------------
Here I think we're validating the correctness of:

  1. Potentially moving 'A' an interleaved load before any store 'B' or
  2. Potentially moving 'B' an interleaved store after any load/store 'A'.

If I am right, I don't think either the comment or the code is tight enough to reflect this.

It would also be good to add testcases for this.

It would be also great to add a testcase to the earlier problem that only candidate accesses were dependency-checked.

What do you think?



http://reviews.llvm.org/D19984