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

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:977
+static bool mustSuppressSpeculation(const LoadInst &LI) {
+  if (!LI.isUnordered())
+    return true;
----------------
MaskRay wrote:
> reames wrote:
> > 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?
> In D87538, `VectorCombine::vectorizeLoadInsert` uses the utility to suppress some cases. It uses `Load->isSimple()` but I don't know whether it should be isUnordered or mustSuppressSpeculation should use isSimple. Appreciate if you can take a look at that piece of code (you can ignore the most complexity there. The main thing is that it is a load widening)
It is safe to speculate an unordered atomic load.

We're conservative in a bunch of places in the optimizer by using isSimple where isUnordered is legal, please don't add new ones if you can avoid it.  


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66688