Andrew Brown (Dec 07 2020 at 20:34): 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?
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.
Chris Fallin (Dec 07 2020 at 20:44):The latter is preferable, yep, and LoadSplat matching in the aarch64 backend is a good example
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
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)
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...
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.
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
Chris Fallin (Dec 07 2020 at 23:21):Sure, happy to look
Chris Fallin (Dec 07 2020 at 23:21):(sorry, was deep in the weeds)
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?
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
Last updated: Oct 23 2024 at 20:03 UTC