8000 Make MachineEnv a per-ABI property by uweigand · Pull Request #6957 · bytecodealliance/wasmtime · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Make MachineEnv a per-ABI property #6957

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 1 commit into from
Sep 12, 2023

Conversation

uweigand
Copy link
Member
@uweigand uweigand commented Sep 2, 2023

The MachineEnv structure contains the allocatable and preferred register sets. This is currently fixed per TargetIsa - however, conceptually these register sets can differ between ABIs on the same ISA.

To allow for this, replace the TargetIsa machine_env routine with an ABIMachineSpec get_machine_env routine. To ensure the structure is still only allocated once, cache it via static OnceLock variables.

No functional change intended.

FYI @cfallin - this is what I suggested in last week's meeting.

@uweigand uweigand requested a review from a team as a code owner September 2, 2023 14:20
@uweigand uweigand requested review from elliottt and removed request for a team September 2, 2023 14:20
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:x64 Issues related to x64 codegen labels Sep 2, 2023
@cfallin cfallin self-requested a review September 12, 2023 00:37
Copy link
Member
@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a bunch!

(stealing review from @elliottt since discussed in previous Cranelift meeting, hope that's ok!)

@cfallin
Copy link
Member
cfallin commented Sep 12, 2023

@uweigand happy to merge once the merge conflict is resolved...

@elliottt elliottt removed their request for review September 12, 2023 16:17
The MachineEnv structure contains the allocatable and preferred
register sets.  This is 
8000
currently fixed per TargetIsa - however,
conceptually these register sets can differ between ABIs on the
same ISA.

To allow for this, replace the TargetIsa machine_env routine with
an ABIMachineSpec get_machine_env routine.  To ensure the structure
is still only allocated once, cache it via static OnceLock variables.

No functional change intended.
@uweigand
Copy link
Member Author

@uweigand happy to merge once the merge conflict is resolved...

I've rebased now.

@cfallin cfallin enabled auto-merge September 12, 2023 18:37
@cfallin cfallin added this pull request to the merge queue Sep 12, 2023
Merged via the queue into bytecodealliance:main with commit 5c8a603 Sep 12, 2023
@uweigand uweigand deleted the abi-machine-env branch September 12, 2023 21:25
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 13, 2023
The MachineEnv structure contains the allocatable and preferred
register sets.  This is currently fixed per TargetIsa - however,
conceptually these register sets can differ between ABIs on the
same ISA.

To allow for this, replace the TargetIsa machine_env routine with
an ABIMachineSpec get_machine_env routine.  To ensure the structure
is still only allocated once, cache it via static OnceLock variables.

No functional change intended.
< 6DD9 !-- -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0