[PATCH] D100585: [ARM][disassembler] Fix incorrect number of operands MCInst generated by the disassembler

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu created this revision.
myhsu added reviewers: t.p.northover, dnsampaio.
Herald added subscribers: danielkiss, hiraditya, kristof.beyls.
myhsu requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Try to fix bug 49974 <https://bugs.llvm.org/show_bug.cgi?id=49974>.

This patch fixes two issues:

1. `BL` does not use predicate (`BL_pred` is the predicate version of `BL`), so we shouldn't add related operands in `DecodeBranchImmInstruction`.
2. Inside `DecodeT2AddSubSPImm`, we shouldn't add predicate operands into the `MCInst` because `ARMDisassembler::AddThumbPredicate` will do that for us. However, we should handle CC-out operand for `t2SUBspImm` and `t2AddspImm`.

I can't think of a way to test this patch because all the existing disassembler tests are based on disassembled assembly code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100585

Files:
  llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp


Index: llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
===================================================================
--- llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
+++ llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
@@ -2676,8 +2676,12 @@
   if (!tryAddingSymbolicOperand(Address, Address + SignExtend32<26>(imm) + 8,
                                 true, 4, Inst, Decoder))
     Inst.addOperand(MCOperand::createImm(SignExtend32<26>(imm)));
-  if (!Check(S, DecodePredicateOperand(Inst, pred, Address, Decoder)))
-    return MCDisassembler::Fail;
+
+  // We already have BL_pred for BL w/ predicate, no need to add addition
+  // predicate opreands for BL
+  if (Inst.getOpcode() != ARM::BL)
+    if (!Check(S, DecodePredicateOperand(Inst, pred, Address, Decoder)))
+      return MCDisassembler::Fail;
 
   return S;
 }
@@ -6670,17 +6674,14 @@
     return MCDisassembler::Fail;
   if (TypeT3) {
     Inst.setOpcode(sign1 ? ARM::t2SUBspImm12 : ARM::t2ADDspImm12);
-    S = 0;
     Inst.addOperand(MCOperand::createImm(Imm12)); // zext imm12
   } else {
     Inst.setOpcode(sign1 ? ARM::t2SUBspImm : ARM::t2ADDspImm);
     if (!Check(DS, DecodeT2SOImm(Inst, Imm12, Address, Decoder))) // imm12
       return MCDisassembler::Fail;
+    if (!Check(DS, DecodeCCOutOperand(Inst, S, Address, Decoder))) // cc_out
+      return MCDisassembler::Fail;
   }
-  if (!Check(DS, DecodeCCOutOperand(Inst, S, Address, Decoder))) // cc_out
-    return MCDisassembler::Fail;
-
-  Inst.addOperand(MCOperand::createReg(0)); // pred
 
   return DS;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100585.337835.patch
Type: text/x-patch
Size: 1559 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210415/ed18c7a4/attachment.bin>