Representing this as 12 separate operations is a bit ugly, but trying to represent the different modes using a bitfield seemed worse.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Representing this as 12 separate operations is a bit ugly, but trying to represent the different modes using a bitfield seemed worse.
Yup, and there's a fairly limited amount of info you can store in the WinEH::Instruction class today, so I guess this looks reasonable.
Technically there's no need to have the assembly level directives mirror exactly the same 12 separate opcodes - we could consider e.g. checking whether the provided register name is in the range of x0-x30, d0-d31 or q0-q31 and pick the right opcode based on that - that would reduce the number of assembly directives down to 4 instead of 12. (That's the only aspect of this which is a public interface.) What do you think of that? Internally throughout the rest of the interfaces (AArch64TargetStreamer) it's probably easiest to keep all 12 separate entrypoints in any case.
llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp | ||
---|---|---|
6104 | Nit: Missing space after == |
That would be four directives, "seh_save_any_reg", seh_save_any_reg_p", "seh_save_any_reg_x", and "seh_save_any_reg_px"? I guess that works, although I'll have to reimplement the parseRegisterInRange() logic.
Exactly.
I'm not entirely dead set on it, but I think it could be a nicer experience for the assembly directives and at least worth evaluating.
LGTM, thanks! This does look nicer to me in the end.
BTW, the process for contributing to the public docs on the unwind info format went really smoothly for me this time, so getting this opcode documented should hopefully go well too.
Nit: Missing space after ==