stack maps -- basically untested? · cranelift

Am I understanding right that cranelift's support for generating stack maps is basically untested right now? I see that we have some test safepoint filetests for exercising the insertion of safepoints, but nothing that exercises the actual generated stack maps. Am I overlooking some tests somewhere?

$ git ls-files cranelift | grep test | xargs grep -rni -E 'stack.?map'
cranelift/codegen/src/isa/test_utils.rs:85:    fn add_stackmap(&mut self, _val_list: &[Value], _func: &Function, _isa: &dyn TargetIsa) {}
cranelift/filetests/src/function_runner.rs:3:use cranelift_codegen::binemit::{NullRelocSink, NullStackmapSink, NullTrapSink};
cranelift/filetests/src/function_runner.rs:270:    let stackmaps = &mut NullStackmapSink {};
cranelift/filetests/src/function_runner.rs:275:        context.emit_to_memory(isa, code_page.as_mut_ptr(), relocs, traps, stackmaps);
cranelift/filetests/src/test_binemit.rs:111:    fn add_stackmap(
cranelift/filetests/src/test_compile.rs:132:    fn add_stackmap(
cranelift/filetests/src/test_rodata.rs:135:    fn add_stackmap(&mut self, _: &[Value], _: &Function, _: &dyn TargetIsa) {}

Andrew Brown (Jun 02 2020 at 23:59):

Benjamin Bouvier (Jun 03 2020 at 16:22):

It's effectively used in Spidermonkey when using Cranelift on x86; there might not be any direct testing in Cranelift, though.

fitzgen (he/him) (Jun 03 2020 at 16:48):

yeah, that only tests the baldrmonkey calling convention, so I think it is worth adding a test stackmaps mode to make it easier to exercise with sys v and windows fastcall etc.

Techcable (Jun 05 2020 at 21:59):

Techcable (Jun 05 2020 at 22:16):

I have a JIT that uses integers for its internal pointers and I'd like to use stackmaps to make deoptimizing back to the interpreter more efficient.

Dan Gohman (Jun 05 2020 at 22:26):

That may be difficult to support. If you give Cranelift integers, it may assume it fully understands them and can do things like change the values of intermediate computations.

Dan Gohman (Jun 05 2020 at 22:28):

Also, if Cranelift doesn't know what's a regular integer versus an integer holding a reference, stack maps would have to hold all integers, which could make them much bigger.

Techcable (Jun 06 2020 at 17:53):

Would it be possible to add an instruction to explicitly create custom stackmaps? My frontend has enough info to handle recovering the interpreter's state.

Techcable (Jun 06 2020 at 17:54):

Right now I'd have to do an explicit branch + call before every trap. This would horribly bloat the code (and presumably ruin register allocation).

Techcable (Jun 06 2020 at 17:56):

If there explicitly requested it wouldn't penalize other uses of cranelift. Also we could refactor the implicit safepoints to use the more general stackmap system.

Stream: cranelift

Topic: stack maps -- basically untested?

fitzgen (he/him) (Jun 02 2020 at 23:28):