[PATCH] D53734: [GlobalISel] Fix a bug in LegalizeRuleSet::clampMaxNumElements

[Volkan Keles via Phabricator via llvm-commits]

volkan added inline comments.


================
Comment at: lib/Target/AArch64/AArch64LegalizerInfo.cpp:173
+      .clampNumElements(0, v2s32, v2s32)
+      .clampMaxNumElements(0, s64, 1);
 
----------------
dsanders wrote:
> 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)
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)
```






================
Comment at: test/CodeGen/AArch64/GlobalISel/legalize-load-fewerElts.mir:5-6
+# 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
+---
----------------
dsanders wrote:
> 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?
That was my plan, but D53728 is required to do that. I'll update the tests in D53728 after committing this.


https://reviews.llvm.org/D53734