AArch64 assembly syntax emission now leverages markup tags for registers, if enabled.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM!
llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp | ||
---|---|---|
1473–1474 | nit: You could write this in fewer lines I think with just: if (MRI.getRegClass(AArch64::ZPRRegClassID).contains(Reg)) printRegName(O, Reg); else printRegName(O, Reg, AArch64::vreg); O << LayoutSuffix; |
@david-arm, thanks for the nit, had missed this. I just updated the diff w/ that change included. Mind committing it on my behalf (GitHub account associated to this profile, as I do not have commit access)? Thank you.
Hmmm sorry, I hadn't seen this when pushing 1b726f0a4ceec0747782545f3d4727f5914ee04b . This needs a rebase and ideally this patch can add a few tests.
@MaskRay, yeah, sorry, I should have probably specified it :/ Essentially, the other commit 4e9907977465 was relying on this one, so this one should have been pushed before, as the other contained the test (which leverages on this change too). Hence, there should be no need to actually introduce tests for this one, as already introduced in the other commit.
You may need to rebase this patch now. I think this changes many instructions not covered by MC/Disassembler/AArch64/marked-up.txt. Having better coverage will be appreicated.
llvm/test/MC/Disassembler/AArch64/marked-up.txt fails.
See Build 280780: pre-merge checks x64 windows failed · x64 debian failed
The last update still failed the test. If the patch has a dependency, you can click "Edit Related Revisions" to state so.
llvm/test/MC/Disassembler/AArch64/marked-up.txt | ||
---|---|---|
16 | It's better to use an instruction family name For st64b, I pick the extension name. |
@MaskRay, fixed by extending properly the patch. Coverage test preserved, and added a further one for completeness.
nit: You could write this in fewer lines I think with just: