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

Daniel Sanders via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 1 10:04:59 PDT 2018


dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM with a couple testing nits



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


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


https://reviews.llvm.org/D53734





More information about the llvm-commits mailing list