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 the info in MCInstrDesc is based on opcodes only, it is often quite
inaccurate. The MCInstrAnalysis has been added so that targets can provide
accurate info, which is based on registers used by the instruction, through
the own versions of MCInstrDesc functions.

The RISCVMCInstrAnalysis, which needs to refine several MCInstrDesc methods,
is a good example for this.

Given the llvm-objdump also uses MCInstrAnalysis, I think this change is in
the right direction.

The default implementation of MCInstrAnalysis methods forward the query to
MCInstrDesc functions. Hence, no functional change is intended/expected.

To avoid bloating up MCInstrAnalysis, only the methods provided by it and
the ones used by disassembler plugin are changed to use MCInstrAnalysis when
available.

Though I am not sure if it will be useful, making MCInstrAnalysis available
in the disassembler plugin would allow enabling symbolize operands feature
in lldb as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156086

Files:
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  llvm/include/llvm/MC/MCInstrAnalysis.h

Index: llvm/include/llvm/MC/MCInstrAnalysis.h
===================================================================
--- llvm/include/llvm/MC/MCInstrAnalysis.h
+++ llvm/include/llvm/MC/MCInstrAnalysis.h
@@ -18,6 +18,7 @@
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCRegisterInfo.h"
 #include <cstdint>
 #include <vector>
 
@@ -64,6 +65,19 @@
     return Info->get(Inst.getOpcode()).isTerminator();
   }
 
+  virtual bool mayAffectControlFlow(const MCInst &Inst,
+                                    const MCRegisterInfo &MCRI) const {
+    if (isBranch(Inst) || isCall(Inst) || isReturn(Inst) ||
+        isIndirectBranch(Inst))
+      return true;
+    unsigned PC = MCRI.getProgramCounter();
+    if (PC == 0)
+      return false;
+    if (Info->get(Inst.getOpcode()).hasDefOfPhysReg(Inst, PC, MCRI))
+      return true;
+    return false;
+  }
+
   /// Returns true if at least one of the register writes performed by
   /// \param Inst implicitly clears the upper portion of all super-registers.
   ///
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===================================================================
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -18,6 +18,7 @@
 #include "llvm/MC/MCDisassembler/MCRelocationInfo.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstPrinter.h"
+#include "llvm/MC/MCInstrAnalysis.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
@@ -75,7 +76,8 @@
                    std::unique_ptr<llvm::MCAsmInfo> &&asm_info_up,
                    std::unique_ptr<llvm::MCContext> &&context_up,
                    std::unique_ptr<llvm::MCDisassembler> &&disasm_up,
-                   std::unique_ptr<llvm::MCInstPrinter> &&instr_printer_up);
+                   std::unique_ptr<llvm::MCInstPrinter> &&instr_printer_up,
+                   std::unique_ptr<llvm::MCInstrAnalysis> &&instr_analysis_up);
 
   std::unique_ptr<llvm::MCInstrInfo> m_instr_info_up;
   std::unique_ptr<llvm::MCRegisterInfo> m_reg_info_up;
@@ -84,6 +86,7 @@
   std::unique_ptr<llvm::MCContext> m_context_up;
   std::unique_ptr<llvm::MCDisassembler> m_disasm_up;
   std::unique_ptr<llvm::MCInstPrinter> m_instr_printer_up;
+  std::unique_ptr<llvm::MCInstrAnalysis> m_instr_analysis_up;
 };
 
 namespace x86 {
@@ -1287,11 +1290,15 @@
   if (!instr_printer_up)
     return Instance();
 
-  return Instance(
-      new MCDisasmInstance(std::move(instr_info_up), std::move(reg_info_up),
-                           std::move(subtarget_info_up), std::move(asm_info_up),
-                           std::move(context_up), std::move(disasm_up),
-                           std::move(instr_printer_up)));
+  // Not all targets may have registered createMCInstrAnalysis().
+  std::unique_ptr<llvm::MCInstrAnalysis> instr_analysis_up(
+      curr_target->createMCInstrAnalysis(instr_info_up.get()));
+
+  return Instance(new MCDisasmInstance(
+      std::move(instr_info_up), std::move(reg_info_up),
+      std::move(subtarget_info_up), std::move(asm_info_up),
+      std::move(context_up), std::move(disasm_up), std::move(instr_printer_up),
+      std::move(instr_analysis_up)));
 }
 
 DisassemblerLLVMC::MCDisasmInstance::MCDisasmInstance(
@@ -1301,13 +1308,15 @@
     std::unique_ptr<llvm::MCAsmInfo> &&asm_info_up,
     std::unique_ptr<llvm::MCContext> &&context_up,
     std::unique_ptr<llvm::MCDisassembler> &&disasm_up,
-    std::unique_ptr<llvm::MCInstPrinter> &&instr_printer_up)
+    std::unique_ptr<llvm::MCInstPrinter> &&instr_printer_up,
+    std::unique_ptr<llvm::MCInstrAnalysis> &&instr_analysis_up)
     : m_instr_info_up(std::move(instr_info_up)),
       m_reg_info_up(std::move(reg_info_up)),
       m_subtarget_info_up(std::move(subtarget_info_up)),
       m_asm_info_up(std::move(asm_info_up)),
       m_context_up(std::move(context_up)), m_disasm_up(std::move(disasm_up)),
-      m_instr_printer_up(std::move(instr_printer_up)) {
+      m_instr_printer_up(std::move(instr_printer_up)),
+      m_instr_analysis_up(std::move(instr_analysis_up)) {
   assert(m_instr_info_up && m_reg_info_up && m_subtarget_info_up &&
          m_asm_info_up && m_context_up && m_disasm_up && m_instr_printer_up);
 }
@@ -1365,6 +1374,8 @@
 
 bool DisassemblerLLVMC::MCDisasmInstance::CanBranch(
     llvm::MCInst &mc_inst) const {
+  if (m_instr_analysis_up)
+    return m_instr_analysis_up->mayAffectControlFlow(mc_inst, *m_reg_info_up);
   return m_instr_info_up->get(mc_inst.getOpcode())
       .mayAffectControlFlow(mc_inst, *m_reg_info_up);
 }
@@ -1375,6 +1386,8 @@
 }
 
 bool DisassemblerLLVMC::MCDisasmInstance::IsCall(llvm::MCInst &mc_inst) const {
+  if (m_instr_analysis_up)
+    return m_instr_analysis_up->isCall(mc_inst);
   return m_instr_info_up->get(mc_inst.getOpcode()).isCall();
 }
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits... Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits
    • [Lldb-co... Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits
    • [Lldb-co... David Spickett via Phabricator via lldb-commits
    • [Lldb-co... Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits
    • [Lldb-co... Jason Molenda via Phabricator via lldb-commits
    • [Lldb-co... Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits
    • [Lldb-co... Jason Molenda via Phabricator via lldb-commits
    • [Lldb-co... Fangrui Song via Phabricator via lldb-commits
    • [Lldb-co... Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits
    • [Lldb-co... Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits
    • [Lldb-co... Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits

Reply via email to