Skip to content

[TableGen] Suppress per-HwMode duplicate instructions/tables. #82567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

nvjle
Copy link
Contributor

@nvjle nvjle commented Feb 22, 2024

Currently, for per-HwMode encoding/decoding, those instructions that do not have a HwMode override are duplicated into the decoder tables for all HwModes. This includes inducing multiple tables for instructions that are otherwise unrelated (e.g., different namespace with no overrides at all).

This patch adds support to suppress instruction and table duplicates. TableGen option "-gen-disassembler --suppress-per-hwmode-duplicates" enables the suppression (off by default).

For one downstream backend with a complicated ISA and major cross-generation encoding differences, this eliminates ~32000 duplicate table entries at the time of this patch.

There are legitimate reasons to suppress or not suppress duplicates. If there are relatively few non-overridden related instructions, it can be convenient to pull them into the per-mode tables (only need to decode the per-mode tables, slightly simpler decode function in disassembler). On the other hand, in some backends, the opposite is true or the size is too large to tolerate any duplication in the first place. We let the user decide which makes sense.

This is currently off by default, though there is no reason it couldn't be enabled by default. Any existing backends downstream using the per-HwMode feature will function as before. Turning on the feature requires minor modifications to their disassembler due to more/less tables and naming.

Currently, for per-HwMode encoding/decoding, those instructions that do
not have a HwMode override are duplicated into the decoder tables for all
HwModes. This includes inducing multiple tables for instructions that are
otherwise unrelated (e.g., different namespace with no overrides at all).

This patch adds support to suppress instruction and table duplicates.
TableGen option "-gen-disassembler --suppress-per-hwmode-duplicates"
enables the suppression (off by default).

For one downstream backend with a complicated ISA and major cross-generation
encoding differences, this eliminates ~32000 duplicate table entries at
the time of this patch.

There are legitimate reasons to suppress or not suppress duplicates. If
there are relatively few non-overridden related instructions, it can be
convenient to pull them into the per-mode tables (only need to decode
the per-mode tables, slightly simpler decode function in dissassembler).
On the other hand, in some backends, the opposite is true or the size
is too large to tolerate any duplication in the first place. We let the
user decide which makes sense.

This is currently off by default, though there is no reason it couldn't
be enabled by default. Any existing backends downstream using the per-HwMode
feature will function as before. Turning on the feature requires minor
modifications to their disassembler due to more/less tables and naming.
@nvjle
Copy link
Contributor Author

nvjle commented Feb 22, 2024

I hope one of @topperc @jyknight @jmolloy @0x59616e @wangpc-pp (based on recent commits or past reviews to the same general area) might review this small patch, thank you.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nvjle
Copy link
Contributor Author

nvjle commented Feb 22, 2024

LGTM

Thank you. Could I trouble you again to merge on my behalf?

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

let InOperandList = (ins i32imm:$factor);
let EncodingInfos = EncodingByHwMode<
[ModeA, ModeB], [fooTypeEncA,
fooTypeEncB]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent this part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@wangpc-pp wangpc-pp merged commit 05af9c8 into llvm:main Feb 22, 2024
@wangpc-pp
Copy link
Contributor

@nvjle You may request a commit access if you are going to contribute more. :-)

@nvjle
Copy link
Contributor Author

nvjle commented Feb 22, 2024

@nvjle You may request a commit access if you are going to contribute more. :-)

💯 Definitely plan to soon-- a few more commits still needed..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants