bytecodealliance / cranelift

Cranelift code generator
https://cranelift.readthedocs.io/
2.49k stars 202 forks source link

add "raw" function definition interface to cranelift-module #1386

Closed froydnj closed 4 years ago

froydnj commented 4 years ago

Feature

While looking at rewriting lucetc to use object instead of faerie (primarily to get Windows support "for free" in lucetc), I think the only reason that "raw" faerie interfaces need to be used instead of going through cranelift-module is that the definition of the stack probe cannot be done as clif:

https://github.com/bytecodealliance/lucet/blob/cf53a3ba5d89174c26ba4ccdd5043628664da72f/lucetc/src/output.rs#L56-L62

as the stack probe must be defined as a certain sequence of instructions that other code needs to know the layout of. And various later things after the above point need to know about the existence of the stack probe, so they can't be written in terms of the cranelift-module interface and need to be written with "raw" faerie.

The straightforward way to rewrite this code (and other code using faerie) to use object is to make the appropriate translations between the two APIs. But there's nothing about the non-stack probe code that cranelift-module can't already handle -- basically defining a bunch of tables with assorted relocations. So I propose adding an interface to cranelift-module::Backend that enables you to define a function in terms of the raw bytes for that function, rather than going through cranelift's code generation.

Benefit

If you squint right, this interface is a primitive form of inline assembly, which has proven useful in other compilers for similar reasons (tightly controlled instruction sequences, as well as access to functionality not directly supported by the compiler or doing things that are Hard To Do in the original language).

For the above example, this interface enables the stack probe to be defined in terms of cranelift-module interfaces, and subsequently all the remaining pieces to be defined in terms of cranelift-module interfaces as well, thus eliminating the need to drop down to a lower-level interface. Switching between faerie and object would get a lot easier as well -- a handful of lines changed, rather than rewriting a bunch of object definitions.

Implementation

This functionality should be straightforward to add: all of the cranelift-module implementations already have to have a way of taking bytes and associating a function name with those bytes, inside Backend::define_function. This interface would just sidestep the code generation parts.

I don't know what the exact signature should look like. Something like define_function_bytes(&mut self, id: FuncId, name: &str, bytes: &[u8], namespace: &ModuleNamespace<Self>) would work for the above use case. Depending on how Right people think a first cut should be, maybe you'd want to include an argument specifying relocations so external functions could be called. I'm inclined to do the simplest thing possible for now.

Alternatives

The alternative that I see is defining some sort of inline assembly bits for the cranelift code generator, which seems like an overwhelmingly large project (although maybe that would have to happen at some point, given potential inline assembly support in rustc?). Maybe there are other alternatives that I haven't thought of.

philipc commented 4 years ago

lucetc needs to also define the trap sites in the stack probe function, so the proposed API doesn't seem to be enough to avoid the raw interface.

But there's nothing about the non-stack probe code that cranelift-module can't already handle -- basically defining a bunch of tables with assorted relocations

The problem is that the function manifest and trap table can't be generated until Module::finish is called because the information they require is in the product.

You're probably aware of this, but I did convert lucetc to use cranelift-object 9 months ago (but it wasn't merged). Is the problem that lucet wants to be able to keep faerie and switch between backends, rather than remove faerie support completely?

froydnj commented 4 years ago

lucetc needs to also define the trap sites in the stack probe function, so the proposed API doesn't seem to be enough to avoid the raw interface.

That's a good point; the interface would also need to specify trap information, which would require extending backends to expose some kind of portable way of expressing trap sites. But that seems pretty straightforward, since cranelift-object and cranelift-faerie do trap collection in very similar ways already. (cranelift-simplejit doesn't deal with traps, so maybe that complicates things a little bit.)

But there's nothing about the non-stack probe code that cranelift-module can't already handle -- basically defining a bunch of tables with assorted relocations

The problem is that the function manifest and trap table can't be generated until Module::finish is called because the information they require is in the product.

The function manifest is defined prior to the conversion out of cranelift-module:

https://github.com/bytecodealliance/lucet/blob/d4b7ffdb9d039e54593561092e487979691cdfdc/lucetc/src/compiler.rs#L181-L195

It does have to be fixed up to reflect the actual length of the stack probe:

https://github.com/bytecodealliance/lucet/blob/d4b7ffdb9d039e54593561092e487979691cdfdc/lucetc/src/output.rs#L63-L78

but that bit would be unnecessary if cranelift-module were aware there was actual code associated with the stack probe.

The actual trap tables lucet needs to define:

https://github.com/bytecodealliance/lucet/blob/d4b7ffdb9d039e54593561092e487979691cdfdc/lucetc/src/traps.rs#L9-L42

can be defined with information retrieved from cranelift-module, assuming that trap site information is exposed in some way.

You're probably aware of this, but I did convert lucetc to use cranelift-object 9 months ago (but it wasn't merged). Is the problem that lucet wants to be able to keep faerie and switch between backends, rather than remove faerie support completely?

My understanding is that it is desirable to keep faerie support since that is a known quantity, and in case problems show up with object. The above proposal is intended to move enough functionality into cranelift-module such that switching between faerie and object is easier than rewriting lucetc/lucetc/src/output.rs and the associated other places that touch faerie internals to support object in parallel.

philipc commented 4 years ago

That's all fine, I'm not opposed to the idea, but it sounded incomplete to me. I did something similar initially but didn't follow through because it wasn't enough benefit, but I didn't go the whole way to try to do the function manifest and trap tables via cranelift-module. For defining the stack probe, my approach was to add an API that accepted a function parameter with a signature similar to Context::emit_to_memory, which lets the caller define relocs/traps/stackmap. The existing Module::define_function could then be implemented using the new API too.

froydnj commented 4 years ago

I realized as I was writing #1400 and experimenting with how it would work with lucet that some sort of API to expose trap information from cranelift-module will also be required...or lucet can have some trait that cranelift-object and cranelift-faerie could implement. Not sure yet which is better.

froydnj commented 4 years ago

I think this was taken care of by #1400. I opened #1404 for exposing trap information in some way.