[PATCH] D94501: [SelectionDAG] Support scalable-vector splats in more cases

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5035
+  } else if (VT.isScalableVector()) {
+    Shift = DAG.getSplatVector(ShVT, dl, Shifts[0]);
+    Factor = DAG.getSplatVector(VT, dl, Factors[0]);
----------------
craig.topper wrote:
> Is there only one element in Shifts and Factors? If I'm reading matchUnaryPredicate right it will only see the single operand of the SPLAT_VECTOR so only call BuildSDIVPattern once?
Yes, that's how it's working. Is that strong enough a thing to rely upon, do you think? 


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5316
   SDValue One = DAG.getConstant(1, dl, VT);
-  SDValue IsOne = DAG.getSetCC(dl, VT, N1, One, ISD::SETEQ);
+  SDValue IsOne = DAG.getSetCC(dl, SetCCVT, N1, One, ISD::SETEQ);
   return DAG.getSelect(dl, VT, IsOne, N0, Q);
----------------
craig.topper wrote:
> I'm a bit surprised this didn't affect AVX512 on X86.
Should we double-check that AVX512 has tested this code path?


================
Comment at: llvm/test/CodeGen/AArch64/sve-int-arith-imm.ll:598
 ; CHECK-NEXT: ret
   %elt = insertelement <vscale x 16 x i8> undef, i8 8, i32 0
   %splat = shufflevector <vscale x 16 x i8> %elt, <vscale x 16 x i8> undef, <vscale x 16 x i32> zeroinitializer
----------------
craig.topper wrote:
> Is this test trying to test an oversized shift amount or just a bad choice of constant? I'm leaning towards bad constant choice the lsl tests don't have this issue?
Yeah I'd imagine the latter too. I tried to tag some of the AArch64 team to see if they could help resolve this issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94501