[PATCH] D109584: [VP] Implementing expansion pass for VP load and store.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/CodeGen/ExpandVectorPredication.cpp:88
+    if (auto *ConstValue = dyn_cast<Constant>(SplattedVal))
+      return ConstValue->isOneValue();
+
----------------
I don't know if it matters whether this is `isOneValue` or `isAllOnesValue`? In practice for `i1` masks it's the same, but somehow the latter sounds more appropriate to me.


================
Comment at: llvm/test/CodeGen/Generic/expand-vp-load-store.ll:34
+; CHECK-NEXT:    [[TMP2:%.*]] = and <2 x i1> [[TMP1]], <i1 true, i1 true>
+; CHECK-NEXT:    [[TMP3:%.*]] = call <2 x i64> @llvm.masked.load.v2i64.p0v2i64(<2 x i64>* [[PTR:%.*]], i32 1, <2 x i1> [[TMP2]], <2 x i64> undef)
+; CHECK-NEXT:    ret <2 x i64> [[TMP3]]
----------------
Shouldn't we see regular `load` here?


================
Comment at: llvm/test/CodeGen/Generic/expand-vp-load-store.ll:78
+; CHECK-NEXT:    [[TMP2:%.*]] = and <2 x i1> [[TMP1]], <i1 true, i1 true>
+; CHECK-NEXT:    call void @llvm.masked.store.v2i64.p0v2i64(<2 x i64> [[VAL:%.*]], <2 x i64>* [[PTR:%.*]], i32 1, <2 x i1> [[TMP2]])
+; CHECK-NEXT:    ret void
----------------
Same here: regular `store`?


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

https://reviews.llvm.org/D109584