[PATCH] D34473: [LV] Changing the interface of ValueMap

Michael Kuperstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 23 16:32:40 PDT 2017


mkuper added a comment.

In https://reviews.llvm.org/D34473#789254, @mssimpso wrote:

> Michael, do you have any issues with moving to this new interface for the ValueMap?


Makes sense to me, modulo a couple of nits.



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:711
+    /// setVectorValue().
+    void unsetVectorValue(Value *Key, unsigned Part) {
+      assert(hasVectorValue(Key, Part) && "Vector value not set for part");
----------------
Since unset is only ever used before set, I think a nicer interface would be to have "reset" that combines an unset with a set.
But if you prefer this, I'm ok with it.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3005
+    Ptr = getOrCreateScalarValue(Ptr, 0, 0);
+  // Else we should vector version of GEP for Gather or Scatter
+  assert((ConsecutiveStride || CreateGatherScatter) &&
----------------
This comment doesn't quite parse.


https://reviews.llvm.org/D34473





More information about the llvm-commits mailing list