[DAGCombine] Check the legality of the index of EXTRACT_SUBVECTOR

[guopeilin]

Summary

For ISD::EXTRACT_SUBVECTOR, its second operand must be a constant
multiple of the known-minimum vector length of the result type.

[david-arm]

llvm/test/CodeGen/AArch64/aarch64-avoid-illegal-extract-subvector.ll

9

Would it be possible to reduce this test a lot further? It looks like it shouldn't be too difficult to hand-write a new test that does something like:

%i1 = extractelement <4 x i32> %x, i32 1
%zi1 = zext i32 %i1 to i64
%i2 = extractelement <4 x i32> %x, i32 2
%zi2 = zext i32 %i2 to i64
%v1 = insertelement <2 x i64> undef, %zi1, i32 0
%v2 = insertelement <2 x i64> %v1, %zi2, i32 1

That looks like what the DAG combine is testing for and since the first extract is at index 1 it is not a multiple of the number of elements in the final build vector (<2 x i64>)

[RKSimon]

llvm/test/CodeGen/AArch64/aarch64-avoid-illegal-extract-subvector.ll

9

@guopeilin Any luck with simplifying this test?

[guopeilin]

Hi all, I update a new patch with a much simple test case. Please review, Thanks a lot!

[guopeilin]

Pinging reviewers....

[RKSimon]

llvm/test/CodeGen/AArch64/aarch64-avoid-illegal-extract-subvector.ll

9

Can this be cleaned up anymore - the dso_local / local_unnamed_addr attributes seem superfluous for instance.

[dmgreen]

Do all the checks pass with this? The precommit checks suggest that an AMD test is failing.

llvm/test/CodeGen/AArch64/aarch64-avoid-illegal-extract-subvector.ll

9

Can you also generate the checks with the script. The file says it's generated with update_llc_test_checks, but these checks look sparse. Can you also add the testcase David talked about:

define <2 x i64> @test(<4 x i32> %x) #0 {
  %i1 = extractelement <4 x i32> %x, i32 1
  %zi1 = zext i32 %i1 to i64
  %i2 = extractelement <4 x i32> %x, i32 2
  %zi2 = zext i32 %i2 to i64
  %v1 = insertelement <2 x i64> undef, i64 %zi1, i32 0
  %v2 = insertelement <2 x i64> %v1, i64 %zi2, i32 1
  ret <2 x i64> %v2
}

It won't fail, but does show the same unaligned EXTRACT_SUBVECTOR. I guess that at some point worked for unaligned extract offsets.

[guopeilin]

Hi all!
Updata a new test case.
Function test2 will fail due to assertion of invalid offset of EXTRACT_SUBVECTOR.
Also modify the AMD test case

[dmgreen]

Thanks. Looks good to me, as per the definition of EXTRACT_SUBVECTOR.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::convertBuildVecZextToZext(SDNode *N) {
 
   // Make sure the first element matches
   // (zext (extract_vector_elt X, C))
   int64_t Offset = checkElem(Op0);
   if (Offset < 0)
     return SDValue();
 
   unsigned NumElems = N->getNumOperands();
+  // Offset must be a constant multiple of the
+  // known-minimum vector length of the result type.
+  if (Offset % NumElems != 0)
+    return SDValue();
+
   SDValue In = Op0.getOperand(0).getOperand(0);
   EVT InSVT = In.getValueType().getScalarType();
   EVT InVT = EVT::getVectorVT(*DAG.getContext(), InSVT, NumElems);
 
   // Don't create an illegal input type after type legalization.
   if (LegalTypes && !TLI.isTypeLegal(InVT))
     return SDValue();
 

llvm/test/CodeGen/AArch64/aarch64-avoid-illegal-extract-subvector.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64-none-linux-gnu < %s -o -| FileCheck %s
+
+@a = external dso_local local_unnamed_addr global i16, align 4
+@f = external dso_local local_unnamed_addr global i32, align 4
+
+define dso_local void @myFunc() local_unnamed_addr {
+; CHECK-LABEL: myFunc:
+; CHECK:       // %bb.0: // %entry
+; CHECK:         addv
+; CHECK-NEXT:    str
+; CHECK-NEXT:    ret
+;
+entry:
+  %f = load i32, i32* @f, align 4
+  %0 = insertelement <8 x i32> zeroinitializer, i32 %f, i32 0
+  %1 = shufflevector <8 x i32> %0, <8 x i32> undef, <8 x i32> zeroinitializer
+  %2 = add <8 x i32> %1, <i32 -4, i32 -8, i32 -12, i32 -16, i32 -20, i32 -24, i32 -28, i32 -32>
+  %3 = extractelement <8 x i32> %2, i32 1
+  %conv18.2 = zext i32 %3 to i64
+  %4 = extractelement <8 x i32> %2, i32 2
+  %conv18.3 = zext i32 %4 to i64
+  %5 = insertelement <8 x i64> zeroinitializer, i64 1, i32 0
+  %6 = insertelement <8 x i64> undef, i64 %conv18.2, i32 2
+  %7 = insertelement <8 x i64> %6, i64 %conv18.3, i32 3
+  %8 = icmp ne <8 x i64> %5, %7
+  %9 = sext <8 x i1> %8 to <8 x i16>
+  %10 = call i16 @llvm.vector.reduce.add.v8i16(<8 x i16> %9)
+  store i16 %10, i16* @a, align 4
+  ret void
+
+}
+
+; Function Attrs: nofree nosync nounwind readnone willreturn
+declare i16 @llvm.vector.reduce.add.v8i16(<8 x i16>)