[PATCH] D96174: [SelectionDAG][AArch64] Restrict matchUnaryPredicate to only handle SPLAT_VECTOR for scalable vectors.

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: frasercrmck, cameron.mcinally, RKSimon, spatel, huntergr.
Herald added subscribers: ecnelises, danielkiss, hiraditya, kristof.beyls.
craig.topper requested review of this revision.
Herald added a project: LLVM.

fde24661718c7812a20a10e518cd853e8e060107 <https://reviews.llvm.org/rGfde24661718c7812a20a10e518cd853e8e060107> added support for
scalable vectors to matchUnaryPredicate by handling SPLAT_VECTOR in
addition to BUILD_VECTOR. This was used to enabled UDIV/SDIV/UREM/SREM
by constant expansion in BuildUDIV/BuildSDIV in TargetLowering.cpp

The caller there expects to call getBuildVector from the match factors.
This leads to a crash right now if there is a SPLAT_VECTOR of
fixed vectors since the number of vectors won't match the number
of elements.

This patch disables matchUnaryPredicate in this case to match
the previous behavior. I'm not sure what the best behavior is
since this function calls a lambda for each element. Should it
call the lambda 1 time for num elements times for a fixed vector?
If the caller is adding constants to vectors like in BuildUDIV,
which behavior we choose has an impact to caller.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96174

Files:
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/test/CodeGen/AArch64/sve-fixed-length-int-div.ll


Index: llvm/test/CodeGen/AArch64/sve-fixed-length-int-div.ll
===================================================================
--- llvm/test/CodeGen/AArch64/sve-fixed-length-int-div.ll
+++ llvm/test/CodeGen/AArch64/sve-fixed-length-int-div.ll
@@ -968,4 +968,20 @@
   ret void
 }
 
+; This used to crash because isUnaryPredicate and BuildUDIV don't know how
+; a SPLAT_VECTOR of fixed vector type should be handled.
+define void @udiv_constantsplat_v8i32(<8 x i32>* %a) #0 {
+; CHECK-LABEL: udiv_constantsplat_v8i32:
+; CHECK: ptrue [[PG:p[0-9]+]].s, vl[[#min(div(VBYTES,4),8)]]
+; CHECK-NEXT: ld1w { [[OP1:z[0-9]+]].s }, [[PG]]/z, [x0]
+; CHECK-NEXT: mov [[OP2:z[0-9]+]].s, #95
+; CHECK-NEXT: udiv [[RES:z[0-9]+]].s, [[PG]]/m, [[OP1]].s, [[OP2]].s
+; CHECK-NEXT: st1w { [[RES]].s }, [[PG]], [x0]
+; CHECK-NEXT: ret
+  %op1 = load <8 x i32>, <8 x i32>* %a
+  %res = udiv <8 x i32> %op1, <i32 95, i32 95, i32 95, i32 95, i32 95, i32 95, i32 95, i32 95>
+  store <8 x i32> %res, <8 x i32>* %a
+  ret void
+}
+
 attributes #0 = { "target-features"="+sve" }
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -306,11 +306,14 @@
     return Match(Cst);
 
   // FIXME: Add support for vector UNDEF cases?
+  // FIXME: Support SPLAT_VECTOR on fixed vectors. Should it call Match once
+  // or NumElements times?
+  EVT VT = Op.getValueType();
   if (ISD::BUILD_VECTOR != Op.getOpcode() &&
-      ISD::SPLAT_VECTOR != Op.getOpcode())
+      !(ISD::SPLAT_VECTOR == Op.getOpcode() && VT.isScalableVector()))
     return false;
 
-  EVT SVT = Op.getValueType().getScalarType();
+  EVT SVT = VT.getScalarType();
   for (unsigned i = 0, e = Op.getNumOperands(); i != e; ++i) {
     if (AllowUndefs && Op.getOperand(i).isUndef()) {
       if (!Match(nullptr))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96174.321862.patch
Type: text/x-patch
Size: 1923 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210205/8c6bd623/attachment.bin>