Stream: git-wasmtime

Topic: wasmtime / Issue #1570 Fix long-range (non-colocated) aar...

Wasmtime GitHub notifications bot (Apr 21 2020 at 19:49):

github-actions[bot] commented on Issue #1570:

Subscribe to Label Action

cc @bnjbvr

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:module"

Thus the following users have been cc'd because of the following labels:

• bnjbvr: cranelift

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Apr 21 2020 at 19:51):

sunfishcode commented on Issue #1570:

At a quick glance, this does indeed look consistent with the intent of the colocated flag.

Wasmtime GitHub notifications bot (Apr 21 2020 at 20:40):

bjorn3 commented on Issue #1570:

As far as I know the colocated flag is meant for functions that will be at a fixed offset from the current function, so they could use PCREL relocations, rather than using a GOT. I don't think it necessarily means that it is "near". For example a fully statically linked binary could always use colocated even when it is 4GB big.

Wasmtime GitHub notifications bot (Apr 21 2020 at 20:57):

cfallin commented on Issue #1570:

the colocated flag is meant for functions that will be at a fixed offset

Hmm -- given that, it seems there isn't a CLIF-level notion of "in the same module"? Perhaps we can add a new bit to calls and symbol_values -- I'm not sure what to call it, but we somehow need to communicate the notion of "call into another function in the same Wasm module, which due to Wasm bytecode size limits should be in range of most RISC ISAs' call instructions".

It seems the effect of colocated is what we want, at least going off of the description: non-colocated implies indirection through a table, while colocated implies direct PC-rel reference.

Alternately, we could consider just doing the right thing and implementing the veneer-insertion linker behavior; that that puts a larger burden on the client, and also breaks invariants around "this blob of machine code from the backend is a fixed-size blob that will never need to be extended with thunks at link time".

Wasmtime GitHub notifications bot (Apr 21 2020 at 20:59):

cfallin edited a comment on Issue #1570:

the colocated flag is meant for functions that will be at a fixed offset

Hmm -- given that, it seems there isn't a CLIF-level notion of "in the same module"? Perhaps we can add a new bit to calls and symbol_values -- I'm not sure what to call it, but we somehow need to communicate the notion of "call into another function in the same Wasm module, which due to Wasm bytecode size limits should be in range of most RISC ISAs' call instructions" (EDIT: for the Wasm use-case, at least; for e.g. AOT-into-object-file use-cases, we just emit the right relocs and everything works).

It seems the effect of colocated is what we want, at least going off of the description: non-colocated implies indirection through a table, while colocated implies direct PC-rel reference.

Alternately, we could consider just doing the right thing and implementing the veneer-insertion linker behavior; that that puts a larger burden on the client, and also breaks invariants around "this blob of machine code from the backend is a fixed-size blob that will never need to be extended with thunks at link time".

Wasmtime GitHub notifications bot (Apr 21 2020 at 21:05):

sunfishcode commented on Issue #1570:

@bjorn3 Ah, that's true. That's what I get for jumping in without full context here :-}.

Wasmtime GitHub notifications bot (Apr 22 2020 at 15:57):

cfallin commented on Issue #1570:

@bnjbvr re:

I agree with the definition of colocated, at least to my understanding

To make sure I understand -- you're agreeing with the initial assertion that colocated seems in practice to mean "reference inside module that can use direct PC-rel references" vs. "reference to another module that needs to go through some sort of indirection / support arbitrary addresses"? Or the later definition clarified by @bjorn3 that it is just "constant PC-rel offset" (but arbitrarily far away)?

I think the basic question is whether we can nudge the definition toward the former -- more of a "same module" vs. "different module" bit, from which we can infer approximate relocation distance (given module size limits), or whether we need another bit / attribute for this.

Thoughts?

Wasmtime GitHub notifications bot (Apr 22 2020 at 16:01):

bnjbvr commented on Issue #1570:

The former, precisely (direct PCRel call vs load from table + indirect call); at least this seems to be the way we set it in Spidermonkey. We could imagine having a different flag for the latter, but I think that's out of scope for this PR.

Wasmtime GitHub notifications bot (Apr 22 2020 at 21:59):

cfallin commented on Issue #1570:

Updated -- just want to make sure we're all OK with the refined meaning of colocated here -- @sunfishcode, OK to do this (with extra doc-comment note on the bool flag definitions) or would you prefer something else?

Wasmtime GitHub notifications bot (Apr 28 2020 at 03:59):

cfallin commented on Issue #1570:

@sunfishcode -- friendly ping, could you verify whether you're OK with this interpretation of colocated?

Wasmtime GitHub notifications bot (May 05 2020 at 00:31):

cfallin commented on Issue #1570:

Rebased and added a more detailed doc comment to the colocated flag, as per a conversation with @sunfishcode just now. Will merge once the tests are green. In the longer term, we'll need to think a bit more about how to support different code models, beyond the simple RelocDistance here; I'll open an issue for that.

Wasmtime GitHub notifications bot (May 05 2020 at 08:28):

bnjbvr commented on Issue #1570:

as per a conversation with @sunfishcode just now.

Where did this conversation happen? I can't find any trace in all the public channels where I'm hanging out. Could the contents of this discussion be summarized somewhere? @sunfishcode @cfallin

Wasmtime GitHub notifications bot (May 05 2020 at 14:29):

cfallin commented on Issue #1570:

Sorry, this was from a 1:1 IM conversation on Zulip, after I had pinged about the above; I should've asked for a comment here for the record!

Here's a transcript:

@sunfishcode: I must apologize, I don't have enough context to answer this, nor time at this moment to context switch and page it in.
@sunfishcode: Potentially related is the concept of "code models", https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html#index-mcmodel_003dsmall
@sunfishcode: although judging by the comments, what Cranelift is doing here is different from any of GCC's defined code models
@sunfishcode: but eventually, I expect Cranelift will need to evolve in the direction of supporting the standard code models, possibly in addition to its own custom ones
@cfallin: OK, no worries. I had been waiting in case you had more to say, but perhaps it's best to merge it with Ben's existing approval, then address the code-model issue later (I can open an issue for it)?
@sunfishcode: Yeah. Ok, what if you added something like this to the comment on the colocated flag:
@sunfishcode: "The exact distance depends on the code model in use. Currently on AArch64 Cranelift uses a custom code model supporting up to +/- byte displacements"
@sunfishcode: or so

Last updated: Nov 22 2024 at 16:03 UTC