aarch64: fix up regalloc2 semantics. #4830
Conversation
github-actions
bot
added
cranelift
| let rn = pretty_print_ireg(rn, size, allocs); | ||
| let rd = pretty_print_ireg(rd.to_reg(), size, allocs); | ||
| let imm = imm.pretty_print(0, allocs); | ||
| format!("{} {}, {}, {}", op_str, rd, rn, imm) |
There was a problem hiding this comment.
Is there a particular reason to deviate from the architecturally-defined disassembly, especially for the simple, single instruction cases? This looks really weird to anyone who is familiar with AArch64 assembly; similarly for the rest of the pretty printing cases you have modified.
This also results in a lot of noise in the diff.
There was a problem hiding this comment.
You mean the extra input register? This is how we've done it across the backends for the SSA-ification of instructions. While it looks a bit odd once regalloc is done and all registers are physical registers, the input and output sides of a "mod" can actually be different vregs (almost always are in fact) pre-regalloc and IMHO that is important to represent in the VCode printout. Otherwise we have hidden aspects of the IR that we can't see when debugging.
There was a problem hiding this comment.
I see, I suppose you regard this as more of some kind of intermediate representation printout than a generated assembly printout, which is pretty much as I do, and that's why I would prefer if all these printouts are as close to the architecturally specified assembly language as possible (possibly making exceptions for complex VCode operations and things that are out of scope of the ISA spec such as how constant literals are represented), especially since the same logic is used by the CLIF compilation tests, where we are looking at the final generated code post-regalloc. However, I understand your use case, so I think it is acceptable to leave it as it is.
There was a problem hiding this comment.
Yeah, it definitely started out that way; actually early on I had been testing the aarch64 backend by feeding the vcode pretty-print into the assembler. But by now the semantics of VCode are not quite the same as machine code and I think it's more important for the textual view to show an accurate representation of the data structure than to try to match an assembler output. We also have clif-util compile -D that disassembles via Capstone for use-cases where we actually really want an independent verification of the machine code output :-)
|
@akirilov-arm updated, thanks! Just the one pending open question above about pretty-printing remains I think. |
This PR removes all uses of modify-operands in the aarch64 backend, replacing them with reused-input operands instead. This has the nice effect of removing a bunch of move instructions and more clearly representing inputs and outputs. This PR also removes the explicit use of pinned vregs in the aarch64 backend, instead using fixed-register constraints on the operands when insts or pseudo-inst sequences require certain registers. This is the second PR in the regalloc-semantics cleanup series; after the remaining backend (s390x) and the ABI code are cleaned up as well, we'll be able to simplify the regalloc2 frontend.
cfallin
force-pushed
the
aarch64-ra2-semantics
branch
from
d53cb3d to
5baa44a
Compare
This PR removes all uses of modify-operands in the aarch64 backend,
replacing them with reused-input operands instead. This has the nice
effect of removing a bunch of move instructions and more clearly
representing inputs and outputs.
This PR also removes the explicit use of pinned vregs in the aarch64
backend, instead using fixed-register constraints on the operands when
insts or pseudo-inst sequences require certain registers.
This is the second PR in the regalloc-semantics cleanup series; after
the remaining backend (s390x) and the ABI code are cleaned up as well,
we'll be able to simplify the regalloc2 frontend.