Alex Crichton (Dec 09 2024 at 20:57): Alex Crichton (Dec 09 2024 at 20:57):I've got a regalloc/cranelift question that others might know better how to debug. I've got a CLIF function which emits xconst8 x23, 0 in pulley bytecode which is storing the immediate 0 in register x23. The register x23 is a callee-save register, though, so it needs to be saved at the beginning of the function. This function, however is only saving x25 for some other reason. Basically I'm not sure why the compute_clobbers function is only returning x25 and not other registers (x23 isn't the only one)
Alex Crichton (Dec 09 2024 at 20:57):my guess is that this is some typo or missing piece by accident in the pulley backend, so I'm mostly curious if anyone else knows how this might come about off the top of their head
Chris Fallin (Dec 09 2024 at 21:01):clobbers are computed by looking at defs on all instructions; is the regalloc metadata correct on xconst8?
Alex Crichton (Dec 09 2024 at 21:02):I double-checked it was using reg_def yeah
Chris Fallin (Dec 09 2024 at 21:02):cool, and then the other piece is ensuring it's actually a callee-save; going to look at ABI code, one sec
Alex Crichton (Dec 09 2024 at 21:03):it's listed in DEFAULT_CALLEE_SAVES on the pulley backend -- I'll note there's a typo in this definition where the float registers are using px_reg instead of pf_reg, but that I don't think should affect this
Alex Crichton (Dec 09 2024 at 21:04):I threw some debugging in OperandCollector::reg_def and it looks like it's only ever called with virtual registers, not physical registers, so that seems a bit suspect
Alex Crichton (Dec 09 2024 at 21:05):one thing I also thought was odd was that in compute_clobbers it iterates over operand_ranges but that set is basically empty
Alex Crichton (Dec 09 2024 at 21:06):it only iterates over the Inst::Args instruction which also seems odd
Chris Fallin (Dec 09 2024 at 21:06):ah, so there isn't a separate collector pass to gather clobbers -- we collect once into the shared operand array, then regalloc works over that, then we iterate over that to gather clobbers (here: https://github.com/bytecodealliance/wasmtime/blob/5dcaa13ced14a4fe135036c82fdc9cb9170b1074/cranelift/codegen/src/machinst/vcode.rs#L661)
Chris Fallin (Dec 09 2024 at 21:06):but: is included_in_clobbers correct for the generated instructions?
Alex Crichton (Dec 09 2024 at 21:06):oh you know
Alex Crichton (Dec 09 2024 at 21:06):ok yep that's it
Alex Crichton (Dec 09 2024 at 21:06):the return value of that is inverted
Alex Crichton (Dec 09 2024 at 21:07):I looked at that earlier and glossed over this by accident
Alex Crichton (Dec 09 2024 at 21:07):t hanks!
Chris Fallin (Dec 09 2024 at 21:07):included_in_clobbers, doing what it says on the tin (but ever sneaky in doing so)! no problem
Alex Crichton (Dec 09 2024 at 21:07):yeah it's defined as is_args() on pulley whereas x64 has !self.is_args() (roughly)
Alex Crichton (Dec 09 2024 at 21:08):all good now though, thanks!
Chris Fallin (Dec 09 2024 at 21:08):yep yep
Alex Crichton (Dec 09 2024 at 21:11):woohoo that got this batch of new tests down to only one failure which I already knew about
Chris Fallin (Dec 09 2024 at 21:11):sweet!
Last updated: Dec 23 2024 at 12:05 UTC