[PATCH] D78776: [llvm-objdump] Print target address with evaluateMemoryOperandAddress()

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp:268-276
     // We only handle PCRel branches for now.
-    if (Info->get(Inst.getOpcode()).OpInfo[0].OperandType!=MCOI::OPERAND_PCREL)
+    if (Inst.getNumOperands() == 0 ||
+        Info->get(Inst.getOpcode()).OpInfo[0].OperandType !=
+            MCOI::OPERAND_PCREL)
       return false;
 
     int64_t Imm = Inst.getOperand(0).getImm();
----------------
skan wrote:
> skan wrote:
> > MaskRay wrote:
> > > skan wrote:
> > > > I have two questions about this change.
> > > > 1. Is there any instruction that does not have any operands for ARM backend?
> > > > 2. If the only difference between the specialized `evaluateBranch` and the gerneral one was that 
> > > > 
> > > > >  In ARM mode the PC is always off by 8 bytes.
> > > > 
> > > > could we use `MCInstrAnalysis::evaluateMemoryOperandAddress(Inst, Addr, 8 , Target)` here?
> > > > 
> > > > 
> > > > Is there any instruction that does not have any operands for ARM backend?
> > > 
> > > Yes, e.g. `nop`
> > > 
> > > > could we use MCInstrAnalysis::evaluateMemoryOperandAddress(Inst, Addr, 8 , Target) here?
> > > 
> > > No. The ARM backend does not derive from `MCInstrAnalysis::evaluateMemoryOperandAddress`.
> > sorry.. I meant 
> > 
> > could we use MCInstrAnalysis::evaluateBranch(Inst, Addr, 8 , Target) here?
> `class ARMMCInstrAnalysis : public MCInstrAnalysis` , ARMMCInstrAnalysis is derived from  `MCInstrAnalysis`.
No. Actually the current `MCInstrAnalysis::evaluateBranch` implementation is X86 specific. See how it evaluates the target address.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78776