[PATCH] D19501: Add LoadStoreVectorizer pass

Justin Lebar via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 17 13:23:05 PDT 2016

jlebar added inline comments.

Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:304
@@ +303,3 @@
+  }
Talking to asbirlea, we are not convinced this is safe.  In particular, we cannot safely reorder across instructions that may read or may write arbitrary memory, nor can we safely reorder loads and stores that are not in the chain but which nonetheless may alias each other.

I think we're safe wrt instructions which may read or write, because these will never appear between a chain's first and last instructions.  (I would feel much better if we had an assert to this effect, though.)

But I am not convinced we're safe wrt regular loads and stores whose addresses depend on the result of a vectorized load.  For example, imagine we have

  b = load a
  load [b]  // b does not alias a
  store [b]
  c = load a+1

Suppose that we choose to vectorize the first and last load and WLOG assume we insert the vectorized load at the position of c:

  load [b]
  store [b]
  {b, c} = load {a, a+1}

Now we call reorder() to fix this up.  The first two instructions are going to be moved below the vectorized load in the reverse order of I->uses().  AFAICT there is no guarantee on the order of uses(), but even if there were, outputting in the opposite order seems probably not right?

Even if that were somehow right, afaict we can make reorder() do arbitrary, unsafe reorderings of loads/stores with the following idiom.

  b = load a
  b1 = b + 0 // copy b to b1
  b2 = b + 0 // copy b to b2
  load [b1]  // b1 does not alias a
  store [b2]  // b2 does not alias a
  c = load a+1

Now if the order of b1 and b2 in the source determines their order in users(), then that controls the order of "load [b1]" and "store [b2]" in the result, even though there's only one correct ordering of that load and store.

So to summarize, it may be my ignorance, but I don't see why we can rely on there being any particular order to users().  But even if users() has some guaranteed order, I don't see how any ordering could guarantee that we never reorder an aliasing load/store pair.


More information about the llvm-commits mailing list