abrown opened Issue #1341:
What is the feature or code improvement you would like to do in Cranelift?
As described in https://github.com/WebAssembly/simd/issues/117#issuecomment-573494583, Wasm shifts do extra runtime work to ensure the shift value is within a certain range. This is extra work that could be removed if we had knowledge that the shift value was constant and in the required range. This knowledge could also be used to lower the Wasm shifts to instructions using an immediate.
What is the value of adding this in Cranelift?
Emit faster code.
Do you have an implementation plan, and/or ideas for data structures or algorithms to use?
I think this type of optimization may apply to more than just Wasm shifts so I hope this approach works for those as well:
- add an
is_constant(v: Value)
function toDataFlowGraph
; initially this could check if the value passed was created by one of the following instructions:iconst
,fconst
,bconst
,vconst
. This could get more complex later but I feel this simple implementation would be a good start.- in
code_translator.rs
, check if the shift valueis_constant
and if it also inside the expected range (e.g. 0-31), lower the Wasm shift toish*_imm
; otherwise, lower toish*
as is done currently.Have you considered alternative implementations? If so, how are they better or worse than your proposal?
No, open to suggestions.
bjorn3 commented on Issue #1341:
https://github.com/bytecodealliance/cranelift/blob/e6e67154c2cd4b68f1045930097df36783cc748c/cranelift-codegen/src/simple_preopt.rs#L560 already performs
shl
/shr
->shl_imm
/shr_imm
when possible ifopt_level=speed
is used.
abrown commented on Issue #1341:
Good to know; hadn't seen that!
abrown closed Issue #1341:
What is the feature or code improvement you would like to do in Cranelift?
As described in https://github.com/WebAssembly/simd/issues/117#issuecomment-573494583, Wasm shifts do extra runtime work to ensure the shift value is within a certain range. This is extra work that could be removed if we had knowledge that the shift value was constant and in the required range. This knowledge could also be used to lower the Wasm shifts to instructions using an immediate.
What is the value of adding this in Cranelift?
Emit faster code.
Do you have an implementation plan, and/or ideas for data structures or algorithms to use?
I think this type of optimization may apply to more than just Wasm shifts so I hope this approach works for those as well:
- add an
is_constant(v: Value)
function toDataFlowGraph
; initially this could check if the value passed was created by one of the following instructions:iconst
,fconst
,bconst
,vconst
. This could get more complex later but I feel this simple implementation would be a good start.- in
code_translator.rs
, check if the shift valueis_constant
and if it also inside the expected range (e.g. 0-31), lower the Wasm shift toish*_imm
; otherwise, lower toish*
as is done currently.Have you considered alternative implementations? If so, how are they better or worse than your proposal?
No, open to suggestions.
abrown commented on Issue #1341:
...but not sure that works for SIMD (which I guess I should have mentioned above).
abrown reopened Issue #1341:
What is the feature or code improvement you would like to do in Cranelift?
As described in https://github.com/WebAssembly/simd/issues/117#issuecomment-573494583, Wasm shifts do extra runtime work to ensure the shift value is within a certain range. This is extra work that could be removed if we had knowledge that the shift value was constant and in the required range. This knowledge could also be used to lower the Wasm shifts to instructions using an immediate.
What is the value of adding this in Cranelift?
Emit faster code.
Do you have an implementation plan, and/or ideas for data structures or algorithms to use?
I think this type of optimization may apply to more than just Wasm shifts so I hope this approach works for those as well:
- add an
is_constant(v: Value)
function toDataFlowGraph
; initially this could check if the value passed was created by one of the following instructions:iconst
,fconst
,bconst
,vconst
. This could get more complex later but I feel this simple implementation would be a good start.- in
code_translator.rs
, check if the shift valueis_constant
and if it also inside the expected range (e.g. 0-31), lower the Wasm shift toish*_imm
; otherwise, lower toish*
as is done currently.Have you considered alternative implementations? If so, how are they better or worse than your proposal?
No, open to suggestions.
bjorn3 commented on Issue #1341:
That's probably the condition at https://github.com/bytecodealliance/cranelift/blob/e6e67154c2cd4b68f1045930097df36783cc748c/cranelift-codegen/src/simple_preopt.rs#L571
abrown commented on Issue #1341:
Yeah, I guess I could add some code to that function to handle vectors but attempt the same type of translations (instead of mucking around in
code_translator.rs
).
alexcrichton transferred Issue #1341:
What is the feature or code improvement you would like to do in Cranelift?
As described in https://github.com/WebAssembly/simd/issues/117#issuecomment-573494583, Wasm shifts do extra runtime work to ensure the shift value is within a certain range. This is extra work that could be removed if we had knowledge that the shift value was constant and in the required range. This knowledge could also be used to lower the Wasm shifts to instructions using an immediate.
What is the value of adding this in Cranelift?
Emit faster code.
Do you have an implementation plan, and/or ideas for data structures or algorithms to use?
I think this type of optimization may apply to more than just Wasm shifts so I hope this approach works for those as well:
- add an
is_constant(v: Value)
function toDataFlowGraph
; initially this could check if the value passed was created by one of the following instructions:iconst
,fconst
,bconst
,vconst
. This could get more complex later but I feel this simple implementation would be a good start.- in
code_translator.rs
, check if the shift valueis_constant
and if it also inside the expected range (e.g. 0-31), lower the Wasm shift toish*_imm
; otherwise, lower toish*
as is done currently.Have you considered alternative implementations? If so, how are they better or worse than your proposal?
No, open to suggestions.
Last updated: Dec 23 2024 at 13:07 UTC