[llvm] [GIsel][AArch64] Legalize <2 x i16> for G_INSERT_VECTOR_ELT (PR #65830)

Amara Emerson via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 11 06:56:28 PDT 2023


aemerson wrote:

> > > > 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?

https://github.com/llvm/llvm-project/pull/65830


More information about the llvm-commits mailing list