[llvm] [NFC][LLVM] Refactor MachineInstr operand accessors (PR #137261)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 24 15:52:25 PDT 2025


https://github.com/jurahul created https://github.com/llvm/llvm-project/pull/137261

- Change several MachineInstr operand accessors to return ArrayRef (const version) or MutableArrayRef (non-const version).
- Returning ArrayRef simplifies implementing some of the other accessor functions.
- Move non-const getMF() to the .cpp file and remove const casting.
- Minor: remove unnecessary {} on `MachineInstrBuilder::add`.

>From d2053ee766822d21759a6bd7c3354e5bb05cb1f4 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Thu, 24 Apr 2025 15:48:07 -0700
Subject: [PATCH] [NFC][LLVM] Refactor MachineInstr operand accessors

- Change several MachineInstr operand accessors to return ArrayRef
  (const version) or MutableArrayRef (non-const version).
- Returning ArrayRef simplifies implementing some of the other
  accessor functions.
- Move non-const getMF() to the .cpp file and remove const casting.
- Minor: remove unnecessary {} on `MachineInstrBuilder::add`.
---
 llvm/include/llvm/CodeGen/MachineInstr.h      | 112 ++++++++----------
 .../llvm/CodeGen/MachineInstrBuilder.h        |   3 +-
 llvm/lib/CodeGen/MachineInstr.cpp             |   2 +
 3 files changed, 52 insertions(+), 65 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index a5d41733085de..f4f8d14179909 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -15,6 +15,7 @@
 #ifndef LLVM_CODEGEN_MACHINEINSTR_H
 #define LLVM_CODEGEN_MACHINEINSTR_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/PointerSumType.h"
 #include "llvm/ADT/ilist.h"
@@ -43,7 +44,6 @@ class Instruction;
 class MDNode;
 class AAResults;
 class BatchAAResults;
-template <typename T> class ArrayRef;
 class DIExpression;
 class DILocalVariable;
 class LiveRegUnits;
@@ -358,10 +358,7 @@ class MachineInstr
   /// Note: this is undefined behaviour if the instruction does not have a
   /// parent.
   const MachineFunction *getMF() const;
-  MachineFunction *getMF() {
-    return const_cast<MachineFunction *>(
-        static_cast<const MachineInstr *>(this)->getMF());
-  }
+  MachineFunction *getMF();
 
   /// Return the asm printer flags bitvector.
   uint8_t getAsmPrinterFlags() const { return AsmPrinterFlags; }
@@ -580,26 +577,16 @@ class MachineInstr
   unsigned getNumOperands() const { return NumOperands; }
 
   /// Returns the total number of operands which are debug locations.
-  unsigned getNumDebugOperands() const {
-    return std::distance(debug_operands().begin(), debug_operands().end());
-  }
+  unsigned getNumDebugOperands() const { return debug_operands().size(); }
 
-  const MachineOperand& getOperand(unsigned i) const {
-    assert(i < getNumOperands() && "getOperand() out of range!");
-    return Operands[i];
-  }
-  MachineOperand& getOperand(unsigned i) {
-    assert(i < getNumOperands() && "getOperand() out of range!");
-    return Operands[i];
-  }
+  const MachineOperand &getOperand(unsigned i) const { return operands()[i]; }
+  MachineOperand &getOperand(unsigned i) { return operands()[i]; }
 
   MachineOperand &getDebugOperand(unsigned Index) {
-    assert(Index < getNumDebugOperands() && "getDebugOperand() out of range!");
-    return *(debug_operands().begin() + Index);
+    return debug_operands()[Index];
   }
   const MachineOperand &getDebugOperand(unsigned Index) const {
-    assert(Index < getNumDebugOperands() && "getDebugOperand() out of range!");
-    return *(debug_operands().begin() + Index);
+    return debug_operands()[Index];
   }
 
   /// Returns whether this debug value has at least one debug operand with the
@@ -667,6 +654,12 @@ class MachineInstr
   unsigned getNumExplicitDefs() const;
 
   /// iterator/begin/end - Iterate over all operands of a machine instruction.
+
+  // The operands must always be in the following order:
+  // - explicit reg defs,
+  // - other explicit operands (reg uses, immediates, etc.),
+  // - implicit reg defs
+  // - implicit reg uses
   using mop_iterator = MachineOperand *;
   using const_mop_iterator = const MachineOperand *;
 
@@ -676,68 +669,61 @@ class MachineInstr
   const_mop_iterator operands_begin() const { return Operands; }
   const_mop_iterator operands_end() const { return Operands + NumOperands; }
 
-  iterator_range<mop_iterator> operands() {
-    return make_range(operands_begin(), operands_end());
+  MutableArrayRef<MachineOperand> operands() {
+    return MutableArrayRef<MachineOperand>(Operands, NumOperands);
   }
-  iterator_range<const_mop_iterator> operands() const {
-    return make_range(operands_begin(), operands_end());
+  ArrayRef<MachineOperand> operands() const {
+    return ArrayRef<MachineOperand>(Operands, NumOperands);
   }
-  iterator_range<mop_iterator> explicit_operands() {
-    return make_range(operands_begin(),
-                      operands_begin() + getNumExplicitOperands());
+  MutableArrayRef<MachineOperand> explicit_operands() {
+    return operands().take_front(getNumExplicitOperands());
   }
-  iterator_range<const_mop_iterator> explicit_operands() const {
-    return make_range(operands_begin(),
-                      operands_begin() + getNumExplicitOperands());
+  ArrayRef<MachineOperand> explicit_operands() const {
+    return operands().take_front(getNumExplicitOperands());
   }
-  iterator_range<mop_iterator> implicit_operands() {
-    return make_range(explicit_operands().end(), operands_end());
+  MutableArrayRef<MachineOperand> implicit_operands() {
+    return operands().drop_front(getNumExplicitOperands());
   }
-  iterator_range<const_mop_iterator> implicit_operands() const {
-    return make_range(explicit_operands().end(), operands_end());
+  ArrayRef<MachineOperand> implicit_operands() const {
+    return operands().drop_front(getNumExplicitOperands());
   }
-  /// Returns a range over all operands that are used to determine the variable
+
+  /// Returns all operands that are used to determine the variable
   /// location for this DBG_VALUE instruction.
-  iterator_range<mop_iterator> debug_operands() {
-    assert((isDebugValueLike()) && "Must be a debug value instruction.");
-    return isNonListDebugValue()
-               ? make_range(operands_begin(), operands_begin() + 1)
-               : make_range(operands_begin() + 2, operands_end());
+  MutableArrayRef<MachineOperand> debug_operands() {
+    assert(isDebugValueLike() && "Must be a debug value instruction.");
+    return isNonListDebugValue() ? operands().take_front(1)
+                                 : operands().drop_front(2);
   }
   /// \copydoc debug_operands()
-  iterator_range<const_mop_iterator> debug_operands() const {
-    assert((isDebugValueLike()) && "Must be a debug value instruction.");
-    return isNonListDebugValue()
-               ? make_range(operands_begin(), operands_begin() + 1)
-               : make_range(operands_begin() + 2, operands_end());
+  ArrayRef<MachineOperand> debug_operands() const {
+    assert(isDebugValueLike() && "Must be a debug value instruction.");
+    return isNonListDebugValue() ? operands().take_front(1)
+                                 : operands().drop_front(2);
   }
-  /// Returns a range over all explicit operands that are register definitions.
+  /// Returns all explicit operands that are register definitions.
   /// Implicit definition are not included!
-  iterator_range<mop_iterator> defs() {
-    return make_range(operands_begin(),
-                      operands_begin() + getNumExplicitDefs());
+  MutableArrayRef<MachineOperand> defs() {
+    return operands().take_front(getNumExplicitDefs());
   }
   /// \copydoc defs()
-  iterator_range<const_mop_iterator> defs() const {
-    return make_range(operands_begin(),
-                      operands_begin() + getNumExplicitDefs());
+  ArrayRef<MachineOperand> defs() const {
+    return operands().take_front(getNumExplicitDefs());
   }
-  /// Returns a range that includes all operands which may be register uses.
+  /// Returns all operands which may be register uses.
   /// This may include unrelated operands which are not register uses.
-  iterator_range<mop_iterator> uses() {
-    return make_range(operands_begin() + getNumExplicitDefs(), operands_end());
+  MutableArrayRef<MachineOperand> uses() {
+    return operands().drop_front(getNumExplicitDefs());
   }
   /// \copydoc uses()
-  iterator_range<const_mop_iterator> uses() const {
-    return make_range(operands_begin() + getNumExplicitDefs(), operands_end());
+  ArrayRef<MachineOperand> uses() const {
+    return operands().drop_front(getNumExplicitDefs());
   }
-  iterator_range<mop_iterator> explicit_uses() {
-    return make_range(operands_begin() + getNumExplicitDefs(),
-                      operands_begin() + getNumExplicitOperands());
+  MutableArrayRef<MachineOperand> explicit_uses() {
+    return explicit_operands().drop_front(getNumExplicitDefs());
   }
-  iterator_range<const_mop_iterator> explicit_uses() const {
-    return make_range(operands_begin() + getNumExplicitDefs(),
-                      operands_begin() + getNumExplicitOperands());
+  ArrayRef<MachineOperand> explicit_uses() const {
+    return explicit_operands().drop_front(getNumExplicitDefs());
   }
 
   using filtered_mop_iterator =
diff --git a/llvm/include/llvm/CodeGen/MachineInstrBuilder.h b/llvm/include/llvm/CodeGen/MachineInstrBuilder.h
index 49d9402d0c273..d4038dcb89b38 100644
--- a/llvm/include/llvm/CodeGen/MachineInstrBuilder.h
+++ b/llvm/include/llvm/CodeGen/MachineInstrBuilder.h
@@ -229,9 +229,8 @@ class MachineInstrBuilder {
   }
 
   const MachineInstrBuilder &add(ArrayRef<MachineOperand> MOs) const {
-    for (const MachineOperand &MO : MOs) {
+    for (const MachineOperand &MO : MOs)
       MI->addOperand(*MF, MO);
-    }
     return *this;
   }
 
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 2bc18001081ea..b7d4abd4b5910 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -755,6 +755,8 @@ const MachineFunction *MachineInstr::getMF() const {
   return getParent()->getParent();
 }
 
+MachineFunction *MachineInstr::getMF() { return getParent()->getParent(); }
+
 MachineInstr *MachineInstr::removeFromParent() {
   assert(getParent() && "Not embedded in a basic block!");
   return getParent()->remove(this);



More information about the llvm-commits mailing list