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

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

RamNalamothu added a comment.

In D156086#4529791 <https://reviews.llvm.org/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 going to be implemented for all targets we support today?

Except few, all the other targets implement MCInstrAnalysis.

> I see them defined for e.g. MIPS and RISCV, but I'm not sure AArch64 does.

AArch64 does implement it.
AArch64 <https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp#L410>
AMDGPU <https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCTargetDesc.cpp#L123>
PowerPC <https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp#L378>
X86 <https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp#L491>

> Does `isBranch` include other variants like `isUnconditionalBranch`?

No. They are implemented as separate methods. You can see that with a full context diff of MCInstrAnalysis.h changes in this revision or MCInstrAnalysis.h <https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/MC/MCInstrAnalysis.h>

> You check if we got an MCInstrAnalysis for this target - do we get none for a target like AArch64 and fall back to our current behavior?

If the target doesn't implement MCInstrAnalysis or implements but doesn't override the methods, we fall back to the current behavior because the default implementation of MCInstrAnalysis methods forward the query to MCInstrDesc functions which the current behavior relies on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156086