[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG254a31273a27: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin (authored by RamNalamothu). Repository: rG LLVM Github

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu added a comment. Thank you @jasonmolenda. If there are no further comments on llvm side, I will land this in a couple of days. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. The lldb side looks good to me, and I think you've addressed the llvm parts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu added a comment. I think I have addressed all the review feedback. Ok to land? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086 ___ lldb-commits

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu marked an inline comment as done. RamNalamothu added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu marked 3 inline comments as done. RamNalamothu added a comment. Thanks for the comments @MaskRay. Comment at: lldb/unittests/Disassembler/RISCV/TestMCDisasmInstanceRISCV.cpp:23 + +class TestMCDisasmInstanceRISCV : public testing::Test { +public:

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu updated this revision to Diff 546688. RamNalamothu added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086 Files:

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

MaskRay added inline comments. Comment at: lldb/unittests/Disassembler/RISCV/TestMCDisasmInstanceRISCV.cpp:23 + +class TestMCDisasmInstanceRISCV : public testing::Test { +public: Place all classes and test methods in an anonymous namespace.

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu updated this revision to Diff 546306. RamNalamothu added a comment. Remove left overs of copy-paste. Apologies for so many updates. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086 Files:

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu updated this revision to Diff 546297. RamNalamothu added a comment. Oops. Didn't notice the changed formatting of the comment after git-clang-format. Fixed the format issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu updated this revision to Diff 546177. RamNalamothu added a comment. Fix RISC-V test name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086 Files:

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu updated this revision to Diff 546173. RamNalamothu added a comment. Herald added subscribers: luke, frasercrmck, sameer.abuasal, Jim, jocewei, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, johnrusso, rbar. Added RISC-V specific test

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu updated this revision to Diff 545753. RamNalamothu added a comment. Add lldb test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086 Files:

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu added a comment. In D156086#4536992 , @jasonmolenda wrote: > In D156086#4530507 , @RamNalamothu > wrote: > >> In D156086#4529791 , @jasonmolenda >> wrote:

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

MaskRay added a comment. It seems that a lldb specific test is needed. Adding a new method to `llvm/include/llvm/MC/MCInstrAnalysis.h` is fine with me, though I haven't checked the semantics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

jasonmolenda added a comment. In D156086#4530507 , @RamNalamothu wrote: > In D156086#4529791 , @jasonmolenda > wrote: > >> >> Does `isBranch` include other variants like `isUnconditionalBranch`? > > No.

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu added a comment. In D156086#4529791 , @jasonmolenda wrote: > I'm not super familiar with the MCInst class from llvm, and hadn't heard of > MCInstrAnalysis. I was looking through the llvm targets - are these > MCInstrAnalysis primitives

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

jasonmolenda added a comment. I'm not super familiar with the MCInst class from llvm, and hadn't heard of MCInstrAnalysis. I was looking through the llvm targets - are these MCInstrAnalysis primitives going to be implemented for all targets we support today? I see them defined for e.g. MIPS

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu marked an inline comment as done. RamNalamothu added inline comments. Comment at: llvm/include/llvm/MC/MCInstrAnalysis.h:80 + } + /// Returns true if at least one of the register writes performed by DavidSpickett wrote: > Does this have test

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

DavidSpickett added a comment. What I understand is that `MCInstrAnalysis` defaults to using `MCInstrDesc`, however not all targets have `MCInstrAnalysis` at all. So we have this awkwardness here with the two pointers, which is fine. Sounds good to me, I am just wondering about the changes

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu updated this revision to Diff 543406. RamNalamothu added a comment. Refer to the commit which added symbolize operands feature in llvm-objdump. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

RamNalamothu created this revision. Herald added subscribers: luismarques, s.egerton, PkmX, simoncook, arichardson. Herald added a project: All. RamNalamothu requested review of this revision. Herald added subscribers: llvm-commits, lldb-commits, wangpc. Herald added projects: LLDB, LLVM. Since