[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