[PATCH] D65189: [MC] Support returning a structured rich disassembly output

Seiya Nuta via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 12 23:26:36 PDT 2019


seiya added a comment.

In D65189#1625303 <https://reviews.llvm.org/D65189#1625303>, @jhenderson wrote:

> You should update your patch title and description too.


What do you think what the patch title should describe?

How about  //"[MC] Support returning marked up ranges in the disassembly"// or //"[MCInstPrinter] Add MarkupStart/MarkupEnd/WithMarkup//?



================
Comment at: llvm/include/llvm/MC/MCInstPrinter.h:80
+  bool Enabled;
+  std::stack<std::vector<MarkupSpan> *> &Spans;
+  MarkupType Type;
----------------
rupprecht wrote:
> When do you need to have multiple Spans on the stack?
> Can you add test coverage for that case?
> When do you need to have multiple Spans on the stack?
The stack has multiple Spans if the marked up ranges are nested:

```
    movq <reg:%rax>, <mem:256(<reg:%rdi>)>
         ^^^^^^^^^^  ^^^^^^^^^----------^^
          1st span    2nd span (*)
                              ^^^^^^^^^^
                               1st span in the (*)'s InnerSpans
 ```

In this example, when we start disassembling `<reg:%rdi>`, the pointer to `InnerSpans` of  the `<mem:...>` span will be appended into the stack.

> Can you add test coverage for that case?
`test/MC/Disassembler/X86/marked-up.txt` includes such a case. We don't add a test in this patch so this should be committed along with D65190, btw.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65189/new/

https://reviews.llvm.org/D65189





More information about the llvm-commits mailing list