[PATCH] D26561: [AArch64] Split 0 vector stores into scalar store pairs.

[Matthias Braun via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:8817
+///
+static SDValue replaceZeroVectorStore(SelectionDAG &DAG, StoreSDNode *St) {
+  SDValue StVal = St->getValue();
----------------
Should use `StoreSDNode &St` as it cannot be nullptr.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:8823
+  int NumVecElts = VT.getVectorNumElements();
+  if (NumVecElts != 4 && NumVecElts != 2)
+    return SDValue();
----------------
Wouldn't we get this without the if for a 3-element vector:
```
stp xzr, xzr, [x0]
str xzr, [x0 + 16]

```
should be a sensible thing to do. Or does this need more work for a future commit?


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:8944
   // those up regresses performance on micro-benchmarks and olden/bh.
-  if (!VT.isVector() || VT.getVectorNumElements() < 2 || VT == MVT::v2i64)
+  if (VT.getVectorNumElements() < 2 || VT == MVT::v2i64)
     return SDValue();
----------------
nothing. (wanted to delete this comment but phabricator seems to be buggy)


================
Comment at: test/CodeGen/AArch64/ldst-opt.ll:1337
+
+define void @merge_zr32(i32* %p) {
+; CHECK-LABEL: merge_zr32:
----------------
A sentence explaining what we are testing would be nice. Same for the next function.


https://reviews.llvm.org/D26561