Skip to content

Add ELF TLS support in new x64 backend.#2540

Merged
cfallin merged 1 commit into
bytecodealliance:mainfrom
cfallin:x64-tls
Jan 18, 2021
Merged

Add ELF TLS support in new x64 backend.#2540
cfallin merged 1 commit into
bytecodealliance:mainfrom
cfallin:x64-tls

Conversation

@cfallin
Copy link
Copy Markdown
Member

@cfallin cfallin commented Jan 4, 2021

Depends on #2538 and includes that commit in this PR.

This follows the implementation in the legacy x86 backend, including
hardcoded sequence that is compatible with what the linker expects. We
could potentially do better here, but it is likely not necessary.

Thanks to @bjorn3 for a bugfix to an earlier version of this.

@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 Jan 4, 2021
@bjorn3
Copy link
Copy Markdown
Contributor

bjorn3 commented Jan 4, 2021

cranelift/codegen/src/legalizer/tls.rs is empty

Comment thread cranelift/codegen/src/isa/x86/enc_tables.rs Outdated
Comment thread cranelift/filetests/filetests/isa/x64/tls_elf.clif
Copy link
Copy Markdown
Contributor

@bjorn3 bjorn3 left a comment

Choose a reason for hiding this comment

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

LGTM with the todo!() added.

Comment thread cranelift/codegen/src/isa/x64/lower.rs Outdated
@cfallin cfallin force-pushed the x64-tls branch 2 times, most recently from d54298f to dd23e50 Compare January 4, 2021 18:49
@cfallin cfallin force-pushed the x64-tls branch 5 times, most recently from 9a865a6 to cdeb4cf Compare January 6, 2021 18:01
Copy link
Copy Markdown
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

Looks good on principle, I wonder if there are ways to minimize code duplication, see below. Thanks!

Comment thread cranelift/codegen/src/isa/x64/inst/mod.rs Outdated
Comment thread cranelift/codegen/src/isa/x64/inst/emit.rs
@cfallin
Copy link
Copy Markdown
Member Author

cfallin commented Jan 14, 2021

Updated, thanks!

Copy link
Copy Markdown
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm!

Comment thread cranelift/codegen/src/isa/x64/inst/mod.rs
This follows the implementation in the legacy x86 backend, including
hardcoded sequence that is compatible with what the linker expects. We
could potentially do better here, but it is likely not necessary.

Thanks to @bjorn3 for a bugfix to an earlier version of this.
@cfallin cfallin merged commit e04e67e into bytecodealliance:main Jan 18, 2021
@cfallin cfallin deleted the x64-tls branch January 18, 2021 07:26
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.

3 participants