Stream: git-wasmtime

Topic: wasmtime / PR #6402 x64: lower `LoadExtName` to a RIP-rel...

Wasmtime GitHub notifications bot (May 17 2023 at 22:56):

abrown opened PR #6402 from abrown:func-addr-lea to bytecodealliance:main:

This change intends to add a new lowering for MachInst::LoadExtName (the pseudo-instruction behind CLIF's func_addr and symbol_addr) that uses RIP-relative addressing when the name is local to the module (colocated). The existing behavior is retained, which currently loads from the global offset table when the is_pic flag is set (a X86GOTPCRel4 relocation) or loads a relocated immediate (an Abs8 relocation). After this change, a lea instruction will calculate a RIP-relative address if the name in question is marked with CLIF's colocated flag.

Why is this necessary? Though some users of Cranelift, like cranelift-jit, understand how to relocate the existing Abs8 relocations (see [CompiledBlob::perform_relocations]), Wasmtime does not. Though technically Wasmtime does apply relocations (see [CodeMemory::apply_relocations]), these are only for libcalls. Wasmtime will attempt to resolve module-local functions, the ones targeted by func_addr, via some internal relocation patching in [ModuleTextBuilder::append_func]. When it realizes it cannot patch one of these functions, it panics because [LabelUse::from_reloc] only understands the X86CallPCRel4 relocation. This prevents the use of func_addr when translating Wasm code. Since no current Wasmtime translation uses func_addr directly, this change is not technically required, but it does feel like the right option to add, normalizing func_addr's behavior for local functions to MachInst::CallKnown's behavior and making more useful for other Cranelift users in the future.

[CompiledBlob::perform_relocations]: https://github.com/bytecodealliance/wasmtime/blob/752c7ea4dd83f9cb247e00cadf9d1958c0bdf9b6/cranelift/jit/src/compiled_blob.rs#L48
[CodeMemory::apply_relocations]: https://github.com/bytecodealliance/wasmtime/blob/752c7ea4dd83f9cb247e00cadf9d1958c0bdf9b6/crates/jit/src/code_memory.rs#L269
[ModuleTextBuilder::append_func]: https://github.com/bytecodealliance/wasmtime/blob/752c7ea4dd83f9cb247e00cadf9d1958c0bdf9b6/crates/cranelift-shared/src/obj.rs#L145
[LabelUse::from_reloc]: https://github.com/bytecodealliance/wasmtime/blob/752c7ea4dd83f9cb247e00cadf9d1958c0bdf9b6/cranelift/codegen/src/isa/x64/inst/mod.rs#L2759

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (May 17 2023 at 22:56):

abrown requested cfallin for a review on PR #6402.

Wasmtime GitHub notifications bot (May 17 2023 at 22:56):

abrown requested wasmtime-compiler-reviewers for a review on PR #6402.

Wasmtime GitHub notifications bot (May 17 2023 at 22:57):

abrown updated PR #6402.

Wasmtime GitHub notifications bot (May 17 2023 at 22:58):

abrown updated PR #6402.

Wasmtime GitHub notifications bot (May 17 2023 at 22:58):

abrown edited PR #6402:

This change intends to add a new lowering for MachInst::LoadExtName (the pseudo-instruction behind CLIF's func_addr and symbol_addr) that uses RIP-relative addressing when the name is local to the module (colocated). The existing behavior is retained, which currently loads from the global offset table when the is_pic flag is set (a X86GOTPCRel4 relocation) or loads a relocated immediate (an Abs8 relocation). After this change, a lea instruction will calculate a RIP-relative address if the name in question is marked with CLIF's colocated flag.

Why is this necessary? Though some users of Cranelift, like cranelift-jit, understand how to relocate the existing Abs8 relocations (see [CompiledBlob::perform_relocations]), Wasmtime does not. Though technically Wasmtime does apply relocations (see [CodeMemory::apply_relocations]), these are only for libcalls. Wasmtime will attempt to resolve module-local functions, the ones targeted by func_addr, via some internal relocation patching in [ModuleTextBuilder::append_func]. When it realizes it cannot patch one of these functions, it panics because [LabelUse::from_reloc] only understands the X86CallPCRel4 relocation. This prevents the use of func_addr when translating Wasm code. Since no current Wasmtime translation uses func_addr directly, this change is not technically required, but it does feel like the right option to add, normalizing func_addr's behavior for local functions to MachInst::CallKnown's behavior and making it more useful for other Cranelift users in the future.

[CompiledBlob::perform_relocations]: https://github.com/bytecodealliance/wasmtime/blob/752c7ea4dd83f9cb247e00cadf9d1958c0bdf9b6/cranelift/jit/src/compiled_blob.rs#L48
[CodeMemory::apply_relocations]: https://github.com/bytecodealliance/wasmtime/blob/752c7ea4dd83f9cb247e00cadf9d1958c0bdf9b6/crates/jit/src/code_memory.rs#L269
[ModuleTextBuilder::append_func]: https://github.com/bytecodealliance/wasmtime/blob/752c7ea4dd83f9cb247e00cadf9d1958c0bdf9b6/crates/cranelift-shared/src/obj.rs#L145
[LabelUse::from_reloc]: https://github.com/bytecodealliance/wasmtime/blob/752c7ea4dd83f9cb247e00cadf9d1958c0bdf9b6/cranelift/codegen/src/isa/x64/inst/mod.rs#L2759

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (May 17 2023 at 23:26):

cfallin submitted PR review:

LGTM, thanks!

Wasmtime GitHub notifications bot (May 18 2023 at 00:02):

cfallin merged PR #6402.

Wasmtime GitHub notifications bot (May 18 2023 at 07:31):

bjorn3 created PR review comment:

There is a cranelift flag to specify if libcalls are colocated or not.

Wasmtime GitHub notifications bot (May 18 2023 at 07:33):

bjorn3 created PR review comment:

Maybe put this before is_pic? All code in the same object file has a fixed relative location with each other.

Last updated: Dec 23 2024 at 12:05 UTC