Stream: cranelift

Topic: Load[32|64]Zero


view this post on Zulip Andrew Brown (Dec 07 2020 at 20:34):

@Julian Seward, @Chris Fallin, @Benjamin Bouvier: Hey! I'm looking at the translation for Load[32|64]Zero and it currently takes two CLIF instructions: a load and then scalar_to_vector. The appropriate lowering for x64 will be a single instruction, MOVSS or MOVSD, so I to either: 1) tell CLIF that scalar_to_vector can accept a memory address as input (or the equivalent in a new CLIF instruction), which implies that the aarch64 lowering might have to change a bit, or 2) figure out how to make load coalescing work for this case. What opinions do you all have? And if option 2 is preferable, can someone point me to an example?

view this post on Zulip Julian Seward (Dec 07 2020 at 20:35):

(2) is the plan going forwards. I can't point you at any example but I'm sure Chris can.

view this post on Zulip Chris Fallin (Dec 07 2020 at 20:44):

The latter is preferable, yep, and LoadSplat matching in the aarch64 backend is a good example

view this post on Zulip Chris Fallin (Dec 07 2020 at 20:45):

basically you just query the lowering context for "source instruction or constant for this arg", and if it's the load that you can merge, then do so

view this post on Zulip Chris Fallin (Dec 07 2020 at 20:45):

(my PR for x64 load-op merging would be a backend-specific example of this for x64)

view this post on Zulip Andrew Brown (Dec 07 2020 at 22:48):

Ok, I tried the load merging but I'm running into the following issue: MOVSS is considered a move by MachInst::is_move and this is correct when the input is a register. I am load merging a load into a scalar_to_vector using MOVSS and the load is sunk by input_to_reg_mem--this MOVSS, when the input is an address, should be considered a load, not a move. But input_to_reg_mem is returning a RegMem::Reg not a RegMem::Mem so MOVSS is being considered a move and being elided...

view this post on Zulip Andrew Brown (Dec 07 2020 at 22:49):

Now I'm looking at get_value_as_source_or_const and trying to understand what is going on.

view this post on Zulip Andrew Brown (Dec 07 2020 at 23:09):

@Chris Fallin, I can put up a draft PR if that will help... perhaps I'm doing something else wrong

view this post on Zulip Chris Fallin (Dec 07 2020 at 23:21):

Sure, happy to look

view this post on Zulip Chris Fallin (Dec 07 2020 at 23:21):

(sorry, was deep in the weeds)

view this post on Zulip Julian Seward (Dec 08 2020 at 06:16):

I thought is_move was related only to register allocation. Kinda surprised it plays any role in insn selection. But maybe I misunderstand?

view this post on Zulip Andrew Brown (Dec 08 2020 at 17:00):

Ok, I opened a draft PR for this and added a comment reproducing the error: https://github.com/bytecodealliance/wasmtime/pull/2489

Like #2480, this should not be merged until #2432 is resolved and x64 Wasm SIMD spec tests are turned back on. The simd_load_zero.wast test is now in-tree since #2481 but will have to be enabled in...

Last updated: Nov 22 2024 at 17:03 UTC