[PATCH] D66688: [LoopVectorize] Leverage speculation safety to avoid masked.loads

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:957
+    Align = DL.getABITypeAlignment(LI->getType());
+  // For the moment, restrict ourselves to the case where the access size is a
+  // multiple of the requested alignment and the base is aligned.  
----------------
Ayal wrote:
> For the moment - as in TODO?
> 
> the access size - of a single element
> 
> Do we (need to) check that "the base is aligned"?
"the base is aligned" is checked within the called function a line below.  


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:977
+static bool mustSuppressSpeculation(const LoadInst &LI) {
+  if (!LI.isUnordered())
+    return true;
----------------
MaskRay wrote:
> Is this isUnordered or isSimple?
> 
> IIUC isSimple is a subset of isUnordered.
Well, it's written as intended, but that might not be what you're trying to ask?  Do you have an example you're wondering about?


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1017
+    // sufficient to ensure it'll never fault within the loop. For the moment,
+    // we restrict this to loads; stores are more complicated due to
+    // concurrency restrictions.
----------------
xbolva00 wrote:
> Do you plan to work on stores?
I am not going to get back to this any time soon.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66688