[Lldb-commits] [PATCH] D69210: [Disassembler] Simplify MCInst predicates

Mon Oct 21 09:58:16 PDT 2019

clayborg added a comment.

In D69210#1715679 <https://reviews.llvm.org/D69210#1715679>, @vsk wrote:

> Hm, this patch is bugging me.
>
> It looks a bit like instructions are still decoded multiple times in different ways (e.g. in the `Decode` and `CalculateMnemonicOperandsAndComment` methods, which both modify `m_opcode`). Any ideas on whether/how to consolidate these?

I am all for anything that will improve efficiency. This class has evolved over time where we started with just the "CalculateMnemonicOperandsAndComment" and then many other features (can branch, etc) were later built into the class. I don't believe instructions are kept around for long so they typically serve one of two purposes:

- disassembly of instruction stream where only CalculateMnemonicOperandsAndComment is needed
- inspection of multiple instructions for stepping looking at can branch and other information requests

So I am not sure the decoded multiple times in different ways is really important unless we do have a costly client that does both CalculateMnemonicOperandsAndComment and inspecting of instruction attributes (can branch, etc). Again, these objects are created, used and discarded currently AFAIK.

================
Comment at: lldb/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:87
+    //   - Is not a call
+    m_does_branch = eLazyBoolYes;
+    m_has_delay_slot = eLazyBoolNo;
----------------
Why init this to eLazyBoolYes?

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

https://reviews.llvm.org/D69210