Skip to content
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

[RTGTest] Add store instructions #8118

Merged
merged 1 commit into from
Jan 31, 2025
Merged

Conversation

maerhart
Copy link
Member

Should we support labels as operands to store instructions? (and consequently also load instructions)

Assemblers typically only support them as replacements for both the immediate AND the base register. Otherwise, the label address needs to be small enough to fit in the immediate or the assembler inserts additional instructions to load the label in a register to compute the offset (I think most assemblers just error out here, e.g., GAS does). So lw ra, data is fine but lw ra, data(zero) is not.

Not supporting them would mean we must manually insert the additional instructions to move a label value into a register and add it to the base register. The front-end could provide a utility function for that. It wouldn't lead to performance-optimal binary but we don't care about runtime performance anyway.

@maerhart maerhart added the RTG Involving the `rtg` dialect label Jan 23, 2025
@maerhart maerhart requested review from youngar and darthscsi January 23, 2025 11:35
@youngar
Copy link
Member

youngar commented Jan 31, 2025

Without really knowing more, it sounds like we can't guarantee the label address is small enough for an immediate and I would rather ban something that has a "random" chance of not working.

@maerhart maerhart force-pushed the maerhart-rtgtest-arithmetic-instructions branch from 98d3abf to e9ca920 Compare January 31, 2025 20:01
Base automatically changed from maerhart-rtgtest-arithmetic-instructions to main January 31, 2025 20:01
@maerhart
Copy link
Member Author

We can guarantee that the label fits into immediate+base register (so the store instruction with label would only have one register plus the label; the assembler inserts additional instructions to first fill the base register and then use that plus the immediate as operands to the actual store.
The other case (where it cannot be guaranteed) isn't really supported by assembler anyway.

So, if we want to support the safe label operand option, we should probably add them as separate operations.

@maerhart maerhart force-pushed the maerhart-rtgtest-store-instructions branch from 88f021a to 20f678e Compare January 31, 2025 20:09
@maerhart maerhart merged commit 336756a into main Jan 31, 2025
3 checks passed
@maerhart maerhart deleted the maerhart-rtgtest-store-instructions branch January 31, 2025 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RTG Involving the `rtg` dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants