Stream: git-cranelift

Topic: cranelift / Issue #1396 Using pinned base registers in Cr...


view this post on Zulip GitHub (Feb 19 2020 at 00:54):

shravanrn opened Issue #1396:

<!-- Please try to describe precisely what you would like to do in Cranelift and/or
expect from it. You can answer the questions below if they're relevant and
delete this text before submitting. Thanks for opening an issue! -->

Feature

I am a PhD student at UCSD investigating a variety of cranelift performance improvements and cranelift Spectre related hardening. So far, these appear very promising and I am hoping to contribute this back to cranelift in the future.

However a key requirement for this work is being able to pin the heap base register in the cranelift compiler. I believe there was support for this earlier, but the feature has since been removed. I am looking for a way to re-enable or re-implement this. I am using cranelift via the Lucet AOT wasm compiler.

Benefit

This will allow for analysis and prototyping of various performance improvements and Spectre related hardening for the cranelift compiler.

Implementation

I believe there was support for this earlier, but this was removed. I am looking for a way to re-enable or re-implement this. A quick and dirty hack for this is fine too, as this is for prototyping, and is not meant for production.

Alternatives

Unfortunately, all of the approaches I am investigating have a concrete reliance on this.

@sunfishcode, @pchickey - We had spoken at the WASM CG about some of this. Would you have thoughts on how best I can proceed here?

view this post on Zulip GitHub (Feb 19 2020 at 00:56):

shravanrn edited Issue #1396:

<!-- Please try to describe precisely what you would like to do in Cranelift and/or
expect from it. You can answer the questions below if they're relevant and
delete this text before submitting. Thanks for opening an issue! -->

Feature

I am a PhD student at UCSD investigating a variety of cranelift performance improvements and cranelift Spectre related hardening. So far, these appear very promising and I am hoping to contribute this back to cranelift in the future.

However a key requirement for this work is being able to pin the heap base register in the cranelift compiler. I believe there was support for this earlier, but the feature has since been removed. I am looking for a way to re-enable or re-implement this. I am using cranelift via the Lucet AOT wasm compiler.

Benefit

This will allow for analysis and prototyping of various performance improvements and Spectre related hardening for the cranelift compiler.

Implementation

I believe there was support for this earlier, but this was removed. I am looking for a way to re-enable or re-implement this. A quick and dirty hack for this is fine too, as this is for prototyping, and is not meant for production.

Alternatives

Unfortunately, all of the approaches I am investigating explicitly rely on pinned base registers.

@sunfishcode, @pchickey - We had spoken at the WASM CG about some of this. Would you have thoughts on how best I can proceed here?

view this post on Zulip GitHub (Feb 19 2020 at 01:01):

iximeow commented on Issue #1396:

So far as I know, Cranelift still supports using a pinned register for the heap base - I just started fixing up a branch in Lucet to expose it as a compilation flag: https://github.com/bytecodealliance/lucet/pull/273

What has you thinking support for this has been removed? If it's missing documentation or something along those lines, we probably ought to fix it :D

view this post on Zulip GitHub (Feb 19 2020 at 01:17):

iximeow edited a comment on Issue #1396:

So far as I know, Cranelift still supports using a pinned register for the heap base - I just started fixing up a branch in Lucet to expose it as a compilation flag: https://github.com/bytecodealliance/lucet/pull/273

What has you thinking support for this has been removed? If it's missing documentation or something along those lines, we probably ought to fix it :D

edit: @pchickey filled me in - it sounds like the current plan is that the new register allocator/backend doesn't have plans to support this from the start? If that's the case consider this a +1 for still wanting this feature, and quietly :eyes: at the conversation here.

view this post on Zulip GitHub (Feb 19 2020 at 02:03):

shravanrn commented on Issue #1396:

@iximeow - thanks for the clarification! Yup, this is indeed a question about both the new and older register allocator. Re the old allocator - I will follow up in the linked lucet PR. Re the new allocator - Yup, I was hoping to start a conversation about supporting this :)

view this post on Zulip GitHub (Feb 19 2020 at 05:12):

lars-t-hansen commented on Issue #1396:

I don't think that we've concluded on this issue - for Firefox, it's really a wasm ABI issue more than a cranelift issue per se. It's believed (based on some preliminary benchmarking) that a pinned heap register is a performance win for many programs, and even when the multi-memory proposal lands I'd be inclined to keep a pinned register for at least one of the memories until we know we're not regressing performance. It's not known how much of win the pinned register is, once we get serious about optimizations that target this particular problem rather than rely on general optimizations & register allocation solutions.

view this post on Zulip GitHub (Feb 19 2020 at 21:58):

shravanrn commented on Issue #1396:

I don't think that we've concluded on this issue... It's not known how much of win the pinned register is...

Ah, gotcha. Thanks for the explanation! Given this, feel free to close this bug as it sounds like discussions on this subject are ongoing.

Just as a summary, the key points I wanted to make from my side was that

  1. Cranelift continues to keep the "pinned & non-spilling" base register support in the current reg allocator
  2. Make the case that Cranelift should continue to support "pinned & non-spilling" base register in the new allocator as well. Our work on security and performance improvements relying on this, while ongoing, have some early results that are promising.

I will try to follow the existing discussions around this feature. Thanks a bunch!

view this post on Zulip GitHub (Feb 21 2020 at 13:15):

bnjbvr commented on Issue #1396:

For what it's worth, the current system allows to pin the heap register via the enable_pinned_reg and use_pinned_reg_as_heap_base settings. Both are false by default. On x86_64, we pin it to r15.

This is definitely something we need to maintain as part of the new regalloc work, as long as Spidermonkey does it, so as to be competitive with Spidermonkey.

view this post on Zulip GitHub (Feb 24 2020 at 03:22):

shravanrn commented on Issue #1396:

Sounds good. Thanks for the info!

view this post on Zulip GitHub (Feb 24 2020 at 03:22):

shravanrn closed Issue #1396:

<!-- Please try to describe precisely what you would like to do in Cranelift and/or
expect from it. You can answer the questions below if they're relevant and
delete this text before submitting. Thanks for opening an issue! -->

Feature

I am a PhD student at UCSD investigating a variety of cranelift performance improvements and cranelift Spectre related hardening. So far, these appear very promising and I am hoping to contribute this back to cranelift in the future.

However a key requirement for this work is being able to pin the heap base register in the cranelift compiler. I believe there was support for this earlier, but the feature has since been removed. I am looking for a way to re-enable or re-implement this. I am using cranelift via the Lucet AOT wasm compiler.

Benefit

This will allow for analysis and prototyping of various performance improvements and Spectre related hardening for the cranelift compiler.

Implementation

I believe there was support for this earlier, but this was removed. I am looking for a way to re-enable or re-implement this. A quick and dirty hack for this is fine too, as this is for prototyping, and is not meant for production.

Alternatives

Unfortunately, all of the approaches I am investigating explicitly rely on pinned base registers.

@sunfishcode, @pchickey - We had spoken at the WASM CG about some of this. Would you have thoughts on how best I can proceed here?


Last updated: Oct 23 2024 at 20:03 UTC