[CodeGen] Modify the refineIndexType(...)'s code to fix a bug in D90942.

[yubing]

Summary

In previous code, when refineIndexType(...) is called and Index is undef, Index.getOperand(0) will raise a assertion fail.

[yubing]

Delete some useles code

[sdesmalen]

Thanks for finding and fixing this!

Can the test be simplified by using <4 x float> instead of <48 x float>?

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

9416

Probably better to explicitly check if Index is not a zero/sign extend, instead of checking whether it has any operands.

[yubing]

Modified some code according to comments

[yubing]

In D92548#2430468, @sdesmalen wrote:

Thanks for finding and fixing this!

Can the test be simplified by using <4 x float> instead of <48 x float>?

Hi, thanks for comments. I can't change into <4 x float>.
I could change to <24 x float> because The smallest type to reproduce this bug is <24 x float>.

In the case of "<24 x float>", the scattered index is represented by a buildvector of<24 x i64> .
During LegalizeType procedure, The scattered index, aka, a buildvector of<24 x i64>will be widened into buildvector (<32 x i64>) whose last 8 elements are undef.
Then, <32 x i64> is split into two buildvectors(16 x i64) , one of which carries 8 undef.
Then, <16 x i64> is split into two buildvectors(8 x i64), one of which carries 8 undef, which means this buildvector will become a v8i64 undef(That's the masked.scatter's index which cause assertion fail).

Any thoughts?

[pengfei]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

9421

How about just use Index = Index.getOperand(0)

[yubing]

Modified some code according Pengfei's comments

[yubing]

PING?

[pengfei]

LGTM.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

bool refineUniformBase(SDValue &BasePtr, SDValue &Index, SelectionDAG &DAG) {
   Index = Index.getOperand(1);
   return true;
 }
 
 // Fold sext/zext of index into index type.
 bool refineIndexType(MaskedScatterSDNode *MSC, SDValue &Index, bool Scaled,
                      SelectionDAG &DAG) {
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
-  SDValue Op = Index.getOperand(0);
 
   if (Index.getOpcode() == ISD::ZERO_EXTEND) {
+    SDValue Op = Index.getOperand(0);
     MSC->setIndexType(Scaled ? ISD::UNSIGNED_SCALED : ISD::UNSIGNED_UNSCALED);
     if (TLI.shouldRemoveExtendFromGSIndex(Op.getValueType())) {
       Index = Op;
       return true;
     }
   }
 
   if (Index.getOpcode() == ISD::SIGN_EXTEND) {
+    SDValue Op = Index.getOperand(0);
     MSC->setIndexType(Scaled ? ISD::SIGNED_SCALED : ISD::SIGNED_UNSCALED);
     if (TLI.shouldRemoveExtendFromGSIndex(Op.getValueType())) {
       Index = Op;
       return true;
     }
   }
 
   return false;

llvm/test/CodeGen/X86/combine-undef-index-mscatter.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx512f | FileCheck %s
+
+define void @main(<24 x float*> %x)
+; CHECK-LABEL: main:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    vmovq %rcx, %xmm0
+; CHECK-NEXT:    vmovq %rdx, %xmm1
+; CHECK-NEXT:    vpunpcklqdq {{.*#+}} xmm0 = xmm1[0],xmm0[0]
+; CHECK-NEXT:    vmovq %rsi, %xmm1
+; CHECK-NEXT:    vmovq %rdi, %xmm2
+; CHECK-NEXT:    vpunpcklqdq {{.*#+}} xmm1 = xmm2[0],xmm1[0]
+; CHECK-NEXT:    vinserti128 $1, %xmm0, %ymm1, %ymm0
+; CHECK-NEXT:    vmovq %r9, %xmm1
+; CHECK-NEXT:    vmovq %r8, %xmm2
+; CHECK-NEXT:    vpunpcklqdq {{.*#+}} xmm1 = xmm2[0],xmm1[0]
+; CHECK-NEXT:    vinserti128 $1, {{[0-9]+}}(%rsp), %ymm1, %ymm1
+; CHECK-NEXT:    vinserti64x4 $1, %ymm1, %zmm0, %zmm0
+; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %zmm1
+; CHECK-NEXT:    vmovups {{[0-9]+}}(%rsp), %zmm2
+; CHECK-NEXT:    kxnorw %k0, %k0, %k1
+; CHECK-NEXT:    vbroadcastf128 {{.*#+}} ymm3 = [8.33005607E-1,8.435871E-1,1.69435993E-1,8.33005607E-1,8.33005607E-1,8.435871E-1,1.69435993E-1,8.33005607E-1]
+; CHECK-NEXT:    # ymm3 = mem[0,1,0,1]
+; CHECK-NEXT:    kxnorw %k0, %k0, %k2
+; CHECK-NEXT:    vscatterqps %ymm3, (,%zmm0) {%k2}
+; CHECK-NEXT:    kxnorw %k0, %k0, %k2
+; CHECK-NEXT:    vscatterqps %ymm3, (,%zmm2) {%k2}
+; CHECK-NEXT:    vscatterqps %ymm3, (,%zmm1) {%k1}
+; CHECK-NEXT:    vzeroupper
+; CHECK-NEXT:    retq
+{
+entry:
+  call void @llvm.masked.scatter.v24f32.v24p0f32(<24 x float> <float 0x3FEAA7FB60000000, float 0x3FEAFEAA60000000, float 0x3FC5B01420000000, float 0x3FEAA7FB60000000, float 0x3FEAA7FB60000000, float 0x3FEAFEAA60000000, float 0x3FC5B01420000000, float 0x3FEAA7FB60000000, float 0x3FEAA7FB60000000, float 0x3FEAFEAA60000000, float 0x3FC5B01420000000, float 0x3FEAA7FB60000000, float 0x3FEAA7FB60000000, float 0x3FEAFEAA60000000, float 0x3FC5B01420000000, float 0x3FEAA7FB60000000, float 0x3FEAA7FB60000000, float 0x3FEAFEAA60000000, float 0x3FC5B01420000000, float 0x3FEAA7FB60000000, float 0x3FEAA7FB60000000, float 0x3FEAFEAA60000000, float 0x3FC5B01420000000, float 0x3FEAA7FB60000000>, <24 x float*> %x, i32 4, <24 x i1> <i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true, i1 true>)
+  ret void
+}
+
+declare void @llvm.masked.scatter.v24f32.v24p0f32(<24 x float>, <24 x float*>, i32 immarg, <24 x i1>)