[PATCH] D141135: [RFC][GlobalISel] Replace the current GlobalISel matcher with a bottom-up matcher

Thu Jan 12 07:12:27 PST 2023

Kai added a comment.

In D141135#4046071 <https://reviews.llvm.org/D141135#4046071>, @aemerson wrote:

> In D141135#4036756 <https://reviews.llvm.org/D141135#4036756>, @Kai wrote:
>
>> - Visit of changed instructions in the Combiner is now done iteratively.
>> - The array with the rules to execute in the generated source is a bit more compact.
>>
>> Running LLVM test suite with `-fglobal-isel` on an AArch64 EC2 instance (2 CPUs, 4GB):
>> Current implementation:  22m43.224s
>> Bottom-up implementation: 22m47.165s
>>
>> Size of the binaries:
>> Current implementation: llc 127.280.024, clang 217.540.416
>> Bottom-up implementation: llc 127.026.960, clang 217.291.456
>
> I haven't had time to properly look at the implementation, but a question on this. When you say "running the test suite", are you talking about building the test suite with -fglobal-isel? Not running the generated code right?

Yes, for the time measurement I referred to building the test suite, as this shows the impact of my code on the compile time. (I also ran the test suite to make sure that there are no regressions.)

> Secondly, you mentioned that this is a drop on replacement for the existing matcher, yet we're seeing some code size differences. Do you know why?

Yes. The matcher my code generates has a simpler structure. It is basically:

- Calculate the match set  number for the current instruction, which is either a constant assignment or a table lookup.
- Translate the match set number to a list of rules to execute, which is another table lookup.
- Run the rules

The current matcher has a couple of `if` and `switch` statements before the rules are run. I noted that the generated tables are much more compact than the code generated from `if` and `switch`. The overall price my implementation pays is a greater construction time in llvm-tblgen.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141135/new/

https://reviews.llvm.org/D141135