[PATCH] D120891: [AArch64] Perform first active true vector combine

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15352
+  auto *Idx = dyn_cast<ConstantSDNode>(N->getOperand(1));
+  if (!Idx || Idx->getZExtValue() != 0 || SetCC.getOpcode() != ISD::SETCC)
+    return SDValue();
----------------
You could use `Idx->isZero()` here.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18329
+  case ISD::EXTRACT_VECTOR_ELT: {
+    SDValue Res = performFirstTrueTestVectorCombine(N, DCI, Subtarget);
+    if (Res == SDValue())
----------------
`performExtractVectorEltCombine` looks like a common placeholder for all related combines so can the call to `performFirstTrueTestVectorCombine` me moved into it?


================
Comment at: llvm/test/CodeGen/AArch64/sve-cmp-folds.ll:56-67
+define void @add_arrays(i32* nocapture %a, <vscale x 4 x i32> %b, <vscale x 4 x i1> %p0, <vscale x 4 x i64> %step) #0 {
+; CHECK-LABEL: add_arrays:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    mov x8, xzr
+; CHECK-NEXT:    cntw x9
+; CHECK-NEXT:    ptrue p1.d
+; CHECK-NEXT:    ptrue p2.s
----------------
This test looks rather more involved than the DAGCombine it's trying to verify.  Why not just:
```
define i1 @foo(<vscale x 4 x float> %a, <vscale x 4 x float> %b) #0 {
  %vcond = fcmp oeq <vscale x 4 x float> %a, %b
  %bit = extractelement <vscale x 4 x i1> %vcond, i64 0
  ret i1 %bit
}
```


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

https://reviews.llvm.org/D120891