Skip to content

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Sep 16, 2025

Move DecodeT2AddrModeImm8 and DecodeT2Imm8 definition before its first use and eliminate the last remaining forward declarations of decode functions.

Work on #156560 : Reorder ARM disassembler decode functions to eliminate forward declarations

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff origin/main HEAD --extensions cpp -- llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.
diff --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
index 8b5e9ba3c..a8c5ec962 100644
--- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
+++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
@@ -3212,18 +3212,18 @@ static DecodeStatus DecodeT2AddrModeImm8(MCInst &Inst, unsigned Val,
 
   // Some instructions always use an additive offset.
   switch (Inst.getOpcode()) {
-    case ARM::t2LDRT:
-    case ARM::t2LDRBT:
-    case ARM::t2LDRHT:
-    case ARM::t2LDRSBT:
-    case ARM::t2LDRSHT:
-    case ARM::t2STRT:
-    case ARM::t2STRBT:
-    case ARM::t2STRHT:
-      imm |= 0x100;
-      break;
-    default:
-      break;
+  case ARM::t2LDRT:
+  case ARM::t2LDRBT:
+  case ARM::t2LDRHT:
+  case ARM::t2LDRSBT:
+  case ARM::t2LDRSHT:
+  case ARM::t2STRT:
+  case ARM::t2STRBT:
+  case ARM::t2STRHT:
+    imm |= 0x100;
+    break;
+  default:
+    break;
   }
 
   if (!Check(S, DecodeGPRRegisterClass(Inst, Rn, Address, Decoder)))

@jurahul jurahul marked this pull request as ready for review September 16, 2025 02:53
@llvmbot
Copy link
Member

llvmbot commented Sep 16, 2025

@llvm/pr-subscribers-backend-arm

Author: Rahul Joshi (jurahul)

Changes

Move DecodeT2AddrModeImm8 and DecodeT2Imm8 definition before its first use and eliminate the last remaining forward declarations of decode functions.

Work on #156560 : Reorder ARM disassembler decode functions to eliminate forward declarations


Full diff: https://github.com/llvm/llvm-project/pull/158767.diff

1 Files Affected:

  • (modified) llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp (+59-65)
diff --git a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
index 1d19bc89ccf92..8b5e9ba3c6d1e 100644
--- a/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
+++ b/llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
@@ -159,12 +159,6 @@ class ARMDisassembler : public MCDisassembler {
 
 } // end anonymous namespace
 
-// Forward declare these because the autogenerated code will reference them.
-// Definitions are further down.
-static DecodeStatus DecodeT2AddrModeImm8(MCInst &Inst, unsigned Val,
-                                         uint64_t Address,
-                                         const MCDisassembler *Decoder);
-
 typedef DecodeStatus OperandDecoder(MCInst &Inst, unsigned Val,
                                     uint64_t Address,
                                     const MCDisassembler *Decoder);
@@ -3181,6 +3175,65 @@ static DecodeStatus DecodeT2LoadShift(MCInst &Inst, unsigned Insn,
   return S;
 }
 
+static DecodeStatus DecodeT2Imm8(MCInst &Inst, unsigned Val, uint64_t Address,
+                                 const MCDisassembler *Decoder) {
+  int imm = Val & 0xFF;
+  if (Val == 0)
+    imm = INT32_MIN;
+  else if (!(Val & 0x100))
+    imm *= -1;
+  Inst.addOperand(MCOperand::createImm(imm));
+
+  return MCDisassembler::Success;
+}
+
+static DecodeStatus DecodeT2AddrModeImm8(MCInst &Inst, unsigned Val,
+                                         uint64_t Address,
+                                         const MCDisassembler *Decoder) {
+  DecodeStatus S = MCDisassembler::Success;
+
+  unsigned Rn = fieldFromInstruction(Val, 9, 4);
+  unsigned imm = fieldFromInstruction(Val, 0, 9);
+
+  // Thumb stores cannot use PC as dest register.
+  switch (Inst.getOpcode()) {
+  case ARM::t2STRT:
+  case ARM::t2STRBT:
+  case ARM::t2STRHT:
+  case ARM::t2STRi8:
+  case ARM::t2STRHi8:
+  case ARM::t2STRBi8:
+    if (Rn == 15)
+      return MCDisassembler::Fail;
+    break;
+  default:
+    break;
+  }
+
+  // Some instructions always use an additive offset.
+  switch (Inst.getOpcode()) {
+    case ARM::t2LDRT:
+    case ARM::t2LDRBT:
+    case ARM::t2LDRHT:
+    case ARM::t2LDRSBT:
+    case ARM::t2LDRSHT:
+    case ARM::t2STRT:
+    case ARM::t2STRBT:
+    case ARM::t2STRHT:
+      imm |= 0x100;
+      break;
+    default:
+      break;
+  }
+
+  if (!Check(S, DecodeGPRRegisterClass(Inst, Rn, Address, Decoder)))
+    return MCDisassembler::Fail;
+  if (!Check(S, DecodeT2Imm8(Inst, imm, Address, Decoder)))
+    return MCDisassembler::Fail;
+
+  return S;
+}
+
 static DecodeStatus DecodeT2LoadImm8(MCInst &Inst, unsigned Insn,
                                      uint64_t Address,
                                      const MCDisassembler *Decoder) {
@@ -3490,18 +3543,6 @@ static DecodeStatus DecodeT2AddrModeImm0_1020s4(MCInst &Inst, unsigned Val,
   return S;
 }
 
-static DecodeStatus DecodeT2Imm8(MCInst &Inst, unsigned Val, uint64_t Address,
-                                 const MCDisassembler *Decoder) {
-  int imm = Val & 0xFF;
-  if (Val == 0)
-    imm = INT32_MIN;
-  else if (!(Val & 0x100))
-    imm *= -1;
-  Inst.addOperand(MCOperand::createImm(imm));
-
-  return MCDisassembler::Success;
-}
-
 template <int shift>
 static DecodeStatus DecodeT2Imm7(MCInst &Inst, unsigned Val, uint64_t Address,
                                  const MCDisassembler *Decoder) {
@@ -3517,53 +3558,6 @@ static DecodeStatus DecodeT2Imm7(MCInst &Inst, unsigned Val, uint64_t Address,
   return MCDisassembler::Success;
 }
 
-static DecodeStatus DecodeT2AddrModeImm8(MCInst &Inst, unsigned Val,
-                                         uint64_t Address,
-                                         const MCDisassembler *Decoder) {
-  DecodeStatus S = MCDisassembler::Success;
-
-  unsigned Rn = fieldFromInstruction(Val, 9, 4);
-  unsigned imm = fieldFromInstruction(Val, 0, 9);
-
-  // Thumb stores cannot use PC as dest register.
-  switch (Inst.getOpcode()) {
-  case ARM::t2STRT:
-  case ARM::t2STRBT:
-  case ARM::t2STRHT:
-  case ARM::t2STRi8:
-  case ARM::t2STRHi8:
-  case ARM::t2STRBi8:
-    if (Rn == 15)
-      return MCDisassembler::Fail;
-    break;
-  default:
-    break;
-  }
-
-  // Some instructions always use an additive offset.
-  switch (Inst.getOpcode()) {
-    case ARM::t2LDRT:
-    case ARM::t2LDRBT:
-    case ARM::t2LDRHT:
-    case ARM::t2LDRSBT:
-    case ARM::t2LDRSHT:
-    case ARM::t2STRT:
-    case ARM::t2STRBT:
-    case ARM::t2STRHT:
-      imm |= 0x100;
-      break;
-    default:
-      break;
-  }
-
-  if (!Check(S, DecodeGPRRegisterClass(Inst, Rn, Address, Decoder)))
-    return MCDisassembler::Fail;
-  if (!Check(S, DecodeT2Imm8(Inst, imm, Address, Decoder)))
-    return MCDisassembler::Fail;
-
-  return S;
-}
-
 template <int shift>
 static DecodeStatus DecodeTAddrModeImm7(MCInst &Inst, unsigned Val,
                                         uint64_t Address,

@jurahul
Copy link
Contributor Author

jurahul commented Sep 23, 2025

@davemgreen @ostannard gentle ping. This is the last one of this nature and I can then close the LLVM issue as well.

Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice. LGTM.

@jurahul jurahul merged commit a3ab719 into llvm:main Sep 23, 2025
12 of 13 checks passed
@jurahul jurahul deleted the arm_disasm_reorder_functions branch September 23, 2025 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants