[GIsel][AArch64] Legalize <2 x i16> for G_INSERT_VECTOR_ELT

[vfdff]

Widen the vector elements to 64 bits to make sure it legal instead by
clamping the number of elements. Depend on D153394.

Fixes #63826

[tschuett]

Do we need to add widenVectorEltsToVectorMinSize to more vector operations?

[vfdff]

Do we need to add widenVectorEltsToVectorMinSize to more vector operations?

Now only do the minimal fix type v2s16.

[aemerson]

Do we need to add widenVectorEltsToVectorMinSize to more vector operations?

Now only do the minimal fix type v2s16.

And why not do widenVectorEltsToVectorMinSize()?

[vfdff]

Do we need to add widenVectorEltsToVectorMinSize to more vector operations?

Now only do the minimal fix type v2s16.

And why not do widenVectorEltsToVectorMinSize()?

Thanks @aemerson. Apply your comment, refactor the widenVectorEltsToVectorMinSize to support the scalar type of Query.Types[1], then pass 1 as the type index to avoid add handling for the destination type in LegalizerHelper.

[aemerson]

Do we need to add widenVectorEltsToVectorMinSize to more vector operations?

Now only do the minimal fix type v2s16.

And why not do widenVectorEltsToVectorMinSize()?

Thanks @aemerson. Apply your comment, refactor the widenVectorEltsToVectorMinSize to support the scalar type of Query.Types[1], then pass 1 as the type index to avoid add handling for the destination type in LegalizerHelper.

Sorry, I hadn't understood your earlier comment about the operand indexes not including the destination. Your earlier change with the LegalizerHelper addition makes sense in that context.

If you bring back that LegalizerHelper change, it seems you don't need the additional parameter to widenVectorEltsToVectorMinSize(). That is, the following patch seems to work on the <2 x s16> test case:

diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index cfb95955d1f8..cfef1cc9c299 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -2496,6 +2496,16 @@ LegalizerHelper::widenScalar(MachineInstr &MI, unsigned TypeIdx, LLT WideTy) {
     return Legalized;
   }
   case TargetOpcode::G_INSERT_VECTOR_ELT: {
+    if (TypeIdx == 0) {
+      Observer.changingInstr(MI);
+      const LLT WideEltTy = WideTy.getElementType();
+
+      widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_ANYEXT);
+      widenScalarSrc(MI, WideEltTy, 2, TargetOpcode::G_ANYEXT);
+      widenScalarDst(MI, WideTy, 0);
+      Observer.changedInstr(MI);
+      return Legalized;
+    }
     if (TypeIdx == 1) {
       Observer.changingInstr(MI);

diff --git a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
index e2df8fb1321d..41433650f7c1 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -732,8 +732,7 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)

   getActionDefinitionsBuilder(G_INSERT_VECTOR_ELT)
       .legalIf(typeInSet(0, {v16s8, v8s8, v8s16, v4s16, v4s32, v2s32, v2s64}))
-      .clampMinNumElements(0, s16, 4)
-      .clampMaxNumElements(0, s16, 8);
+      .widenVectorEltsToVectorMinSize(0, 64);

   getActionDefinitionsBuilder(G_BUILD_VECTOR)
       .legalFor({{v8s8, s8},

What do you think?

[vfdff]

I roll back to avoid additional parameter to widenVectorEltsToVectorMinSize(), thanks very much for detail suggestion.

[aemerson]

Thanks, LGTM.