[GlobalISel] Fix a bug in LegalizeRuleSet::clampMaxNumElements

[volkan]

Summary

This function was causing a crash when MaxElements == 1 because
it was trying to create a single element vector type.

[dsanders]

LGTM with a couple testing nits

lib/Target/AArch64/AArch64LegalizerInfo.cpp

173 ↗

(On Diff #171202)

It's not for this patch, but we ought to have a version of clampMaxNumElements() that applies to every vector except for those on a whitelist. Maybe, something like:

.clampMaxNumElements(0, except({s32}), 1)

test/CodeGen/AArch64/GlobalISel/legalize-load-fewerElts.mir

5–6 ↗

(On Diff #171202)

It's better to test the end result rather than the particular action the legalizer takes for this step. Can you drop the -debug-only=legalizer from the run line and test with the output you get without that?

[volkan]

lib/Target/AArch64/AArch64LegalizerInfo.cpp

173 ↗

(On Diff #171202)

I agree. I think we should add the opposite as well, a function that takes a list of types. For example:

.clampMaxNumElements(0, {s16, s32, s64, ...}, 1)

test/CodeGen/AArch64/GlobalISel/legalize-load-fewerElts.mir

5–6 ↗

(On Diff #171202)

That was my plan, but D53728 is required to do that. I'll update the tests in D53728 after committing this.

[dsanders]

test/CodeGen/AArch64/GlobalISel/legalize-load-fewerElts.mir

5–6 ↗

(On Diff #171202)

Ok. That sounds good to me

llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h

return actionIf(
         LegalizeAction::FewerElements,
         [=](const LegalityQuery &Query) {
           LLT VecTy = Query.Types[TypeIdx];
           return VecTy.isVector() && VecTy.getElementType() == EltTy &&
                  VecTy.getNumElements() > MaxElements;
         },
         [=](const LegalityQuery &Query) {
           LLT VecTy = Query.Types[TypeIdx];
+          if (MaxElements == 1)
+            return std::make_pair(TypeIdx, VecTy.getElementType());
           return std::make_pair(
               TypeIdx, LLT::vector(MaxElements, VecTy.getScalarSizeInBits()));
         });
   }
   /// Limit the number of elements for the given vectors to at least MinTy's
   /// number of elements and at most MaxTy's number of elements.
   ///
   /// No effect if the type is not a vector or does not have the same element

llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp

getActionDefinitionsBuilder(G_LOAD)
       .widenScalarToNextPow2(0)
       // TODO: We could support sum-of-pow2's but the lowering code doesn't know
       //       how to do that yet.
       .unsupportedIfMemSizeNotPow2()
       // Lower any any-extending loads left into G_ANYEXT and G_LOAD
       .lowerIf([=](const LegalityQuery &Query) {
         return Query.Types[0].getSizeInBits() != Query.MMODescrs[0].SizeInBits;
       })
-      .clampNumElements(0, v2s32, v2s32);
+      .clampNumElements(0, v2s32, v2s32)
+      .clampMaxNumElements(0, s64, 1);
 
   getActionDefinitionsBuilder(G_STORE)
       .legalForTypesWithMemSize({{s8, p0, 8},
                                  {s16, p0, 16},
                                  {s32, p0, 32},
                                  {s64, p0, 64},
                                  {p0, p0, 64},
                                  {v2s32, p0, 64}})
       .clampScalar(0, s8, s64)
       .widenScalarToNextPow2(0)
       // TODO: We could support sum-of-pow2's but the lowering code doesn't know
       //       how to do that yet.
       .unsupportedIfMemSizeNotPow2()
       .lowerIf([=](const LegalityQuery &Query) {
         return Query.Types[0].isScalar() &&
                Query.Types[0].getSizeInBits() != Query.MMODescrs[0].SizeInBits;
       })
-      .clampNumElements(0, v2s32, v2s32);
+      .clampNumElements(0, v2s32, v2s32)
+      .clampMaxNumElements(0, s64, 1);
 
   // Constants
   getActionDefinitionsBuilder(G_CONSTANT)
       .legalFor({p0, s32, s64})
       .clampScalar(0, s32, s64)
       .widenScalarToNextPow2(0);
   getActionDefinitionsBuilder(G_FCONSTANT)
       .legalFor({s32, s64})

llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-load-fewerElts.mir

+# RUN: llc -march=aarch64 -o - -run-pass=legalizer -global-isel-abort=0 -debug-only=legalizer 2>&1 %s | FileCheck %s
+# REQUIRES: asserts
+
+# CHECK: Legalize Machine IR for: load_v4s32
+# CHECK-NEXT: %{{[0-9]+}}:_(<4 x s32>) = G_LOAD %{{[0-9]+}}:_(p0)
+# CHECK-NEXT: Reduce number of elements
+---
+name:            load_v4s32
+legalized:       false
+tracksRegLiveness: true
+body:             |
+  bb.1:
+    liveins: $x0
+
+    %0:_(p0) = COPY $x0
+    %1:_(<4 x s32>) = G_LOAD %0(p0) :: (load 16, align 4)
+    %2:_(s32), %3:_(s32), %4:_(s32), %5:_(s32) = G_UNMERGE_VALUES %1(<4 x s32>)
+    $w0 = COPY %5(s32)
+
+...
+
+# Make sure we are able to scalarize v2s64.
+# CHECK: Legalize Machine IR for: load_v2s64
+# CHECK-NEXT: %{{[0-9]+}}:_(<2 x s64>) = G_LOAD %{{[0-9]+}}:_(p0)
+# CHECK-NEXT: Reduce number of elements
+---
+name:            load_v2s64
+legalized:       false
+tracksRegLiveness: true
+body:             |
+  bb.1:
+    liveins: $x0
+
+    %0:_(p0) = COPY $x0
+    %1:_(<2 x s64>) = G_LOAD %0(p0) :: (load 16)
+    %2:_(s64), %3:_(s64) = G_UNMERGE_VALUES %1(<2 x s64>)
+    $x0 = COPY %3(s64)
+
+...

llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-load-v4s32.mir

-# RUN: not llc -march=aarch64 -o - -run-pass=legalizer -debug-only=legalizer 2>&1 %s | FileCheck %s
-# REQUIRES: asserts
-
-# CHECK: Legalize Machine IR for: load_v4s32
-# CHECK-NEXT: %{{[0-9]+}}:_(<4 x s32>) = G_LOAD %{{[0-9]+}}:_(p0)
-# CHECK-NOT: Lower
-# CHECK: unable to legalize instruction
----
-name:            load_v4s32
-legalized:       false
-tracksRegLiveness: true
-body:             |
-  bb.1:
-    liveins: $x0
-
-    %0:_(p0) = COPY $x0
-    %1:_(<4 x s32>) = G_LOAD %0(p0) :: (load 16, align 4)
-    %2:_(s32), %3:_(s32), %4:_(s32), %5:_(s32) = G_UNMERGE_VALUES %1(<4 x s32>)
-    $w0 = COPY %5(s32)
-
-...