[PATCH] D129868: [SystemZInstPrinter] Introduce markup tags emission
Ulrich Weigand via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 29 06:50:52 PDT 2022
uweigand added a comment.
In D129868#3751791 <https://reviews.llvm.org/D129868#3751791>, @antoniofrighetto wrote:
> @uweigand, mostly third-party clients such as disassemblers and pretty printers, as per official documentation <https://releases.llvm.org/5.0.2/docs/MarkedUpDisassembly.html>. Already supported by ARM as well as x64.
Do you have any example of such tools? The reason I'm asking is that it's a bit hard for me to judge whether the implementation is fully complete and would make those uses cases actually work, without seeing any.
As a comment on the implementation: I think it would be much preferable to use the base class `UseMarkup` member (or even better, the `markup` helper), instead of having to explicitly pass on the extra argument. I guess the reason you're not doing this is because of the calls from `PrintAsmOperand` / `PrintMemAsmOperand`? This actually makes me wonder if it wouldn't be cleaner to just duplicate the (small amount of) code in `printOperand` and `printAddress` into `PrintAsmOperand` and `PrintMemAsmOperand`, and then stop making all those `SystemZInstrPrinter` routines `static` ...
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129868/new/
https://reviews.llvm.org/D129868
More information about the llvm-commits
mailing list