Work on DAG Isel for TableGen and compiler

I have been working on improvements to TableGen's DAG Isel matcher backend. This has led me to thinking about ways to speed up the compile-time interpreter of the instruction selection matcher table.

Is this worth my time, given Fast Isel and the upcoming Global Isel selector?

Hi Paul,

I think this would be time well spent. At least in the WebAssembly backend, the vast majority of our ISel work is still done with DAG ISel. I know this is different from the performance work you have in mind, but one of my biggest pain points working on LLVM so far has been the poor error messages from the DAG ISel pattern type checker. If you could find time to improve those error messages, I would be extremely grateful.

Thanks,

Thomas

GlobalISel’s matcher table is in need of more optimization work. I think it’s important to actually start shifting effort to working on GlobalISel. At the current rate we’ll never reach a point where we can really switch. GlobalISel is usable now, and I hate to see effort continuing on the legacy path.

-Matt

Are you talking about the type checking done in CodeGenDAGPatterns.cpp? Is it easy to post an example?

Yes, the CodeGenDAGPatterns is exactly right. Try applying the patch below and rebuilding and you’ll see what I mean about the error messages :wink: That being said, I’m sympathetic to Matt’s point about shifting effort to GlobalISel. Maybe it has similar problems you could work on? A nicer development experience would certainly be a good carrot to get me excited to switch over sooner.

— a/llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
@@ -413,7 +413,7 @@ defm SHUFFLE :
def wasm_shuffle_t : SDTypeProfile<1, 18, []>;
def wasm_shuffle : SDNode<“WebAssemblyISD::SHUFFLE”, wasm_shuffle_t>;
foreach vec_t = [v16i8, v8i16, v4i32, v2i64, v4f32, v2f64] in {
-def : Pat<(vec_t (wasm_shuffle (vec_t V128:$x), (vec_t V128:$y),
+def : Pat<(vec_t (wasm_shuffle V128:$x, (vec_t V128:$y),
(i32 LaneIdx32:$m0), (i32 LaneIdx32:$m1),
(i32 LaneIdx32:$m2), (i32 LaneIdx32:$m3),
(i32 LaneIdx32:$m4), (i32 LaneIdx32:$m5),

Given that I'm only somewhat up-to-speed on the DAG ISel scheme and not much at all on the Global ISel scheme, I'm tempted to work on the former and then the latter. So I'll look at the CodeGenDAGPatterns messages first. Then I will take a look at Global ISel.

Matt: Can you suggest one or two things about Global ISel that could use some work? I won't get to it quickly, but it will give me something to focus on.

I would like to know why this patch https://reviews.llvm.org/D90973 had such a drastic size effect on the global isel tablegened matcher table for riscv. It only changed the DAG ISel table by about 15K. But the global isel table shrinks by over 200K.

I can answer that.

The way HwModes are handled is that they are translated into macher’s predicates. Same selection pattern may infer different value types for different modes, and all these variants are then predicated on the condition defining the mode which was applied to produce the variant. There is not much there to common out, because the created variants are really different patterns at this moment.

With the hw modes you can write one pattern in the .td file and get it automatically expanded into variants instead of having to write the variants yourself, so in that sense you still get the large matcher table one way or the other. Now, for the DefaultMode that your patch removes—yes, that’s both a shortcoming of the implementation (that it’s not specified in some other way), and that of the Hexagon .td files (that it duplicated another mode), which inspired the RISCV’s usage.