[llvm] 245e3dd - [MC] Do not copy MCInstrDescs. NFC.

Jay Foad via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 23 03:55:55 PST 2023


Author: Jay Foad
Date: 2023-01-23T11:55:49Z
New Revision: 245e3dd94851affe5f07d50dba7315f5faa500a2

URL: https://github.com/llvm/llvm-project/commit/245e3dd94851affe5f07d50dba7315f5faa500a2
DIFF: https://github.com/llvm/llvm-project/commit/245e3dd94851affe5f07d50dba7315f5faa500a2.diff

LOG: [MC] Do not copy MCInstrDescs. NFC.

Avoid copying MCInstrDesc instances because a future patch will change
them to find their implicit operands and operand info array based on
their own "this" pointer, so it will only work for MCInstrDescs in the
TargetInsts table, not for a copy of an MCInstrDesc at a different
address.

Differential Revision: https://reviews.llvm.org/D142214

Added: 
    

Modified: 
    llvm/include/llvm/MC/MCInstrDesc.h
    llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
    llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
    llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
    llvm/lib/Target/AMDGPU/SIInstrInfo.h
    llvm/lib/Target/Lanai/LanaiDelaySlotFiller.cpp
    llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
    llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVMCCodeEmitter.cpp
    llvm/unittests/CodeGen/LexicalScopesTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/MC/MCInstrDesc.h b/llvm/include/llvm/MC/MCInstrDesc.h
index 4aa568d345e0d..771df1975b89b 100644
--- a/llvm/include/llvm/MC/MCInstrDesc.h
+++ b/llvm/include/llvm/MC/MCInstrDesc.h
@@ -197,6 +197,14 @@ enum Flag {
 /// directly to describe itself.
 class MCInstrDesc {
 public:
+  // Do not allow MCInstrDescs to be copied or moved. They should only exist in
+  // the <Target>Insts table because they rely on knowing their own address to
+  // find other information elsewhere in the same table.
+  MCInstrDesc(const MCInstrDesc &) = delete;
+  MCInstrDesc(MCInstrDesc &&) = delete;
+  MCInstrDesc &operator=(const MCInstrDesc &) = delete;
+  MCInstrDesc &operator=(MCInstrDesc &&) = delete;
+
   unsigned short Opcode;         // The opcode number
   unsigned short NumOperands;    // Num of args (may be more if variable_ops)
   unsigned char NumDefs;         // Num of args that are definitions

diff  --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
index 71abab3b4628b..24837f479d880 100644
--- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
@@ -340,7 +340,7 @@ static void GetCostForDef(const ScheduleDAGSDNodes::RegDefIter &RegDefPos,
     }
 
     unsigned Idx = RegDefPos.GetIdx();
-    const MCInstrDesc Desc = TII->get(Opcode);
+    const MCInstrDesc &Desc = TII->get(Opcode);
     const TargetRegisterClass *RC = TII->getRegClass(Desc, Idx, TRI, MF);
     assert(RC && "Not a valid register class");
     RegClass = RC->getID();

diff  --git a/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp b/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
index f6d672938b627..42d1f58e4239c 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
@@ -2938,7 +2938,7 @@ bool AMDGPUDAGToDAGISel::isVGPRImm(const SDNode * N) const {
       SDNode * User = *U;
       if (User->isMachineOpcode()) {
         unsigned Opc = User->getMachineOpcode();
-        MCInstrDesc Desc = SII->get(Opc);
+        const MCInstrDesc &Desc = SII->get(Opc);
         if (Desc.isCommutable()) {
           unsigned OpIdx = Desc.getNumDefs() + U.getOperandNo();
           unsigned CommuteIdx1 = TargetInstrInfo::CommuteAnyOperandIndex;

diff  --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 4f7e7dad7b277..8fe134705e2a2 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -2452,7 +2452,8 @@ bool SIInstrInfo::findCommutedOpIndices(const MachineInstr &MI,
   return findCommutedOpIndices(MI.getDesc(), SrcOpIdx0, SrcOpIdx1);
 }
 
-bool SIInstrInfo::findCommutedOpIndices(MCInstrDesc Desc, unsigned &SrcOpIdx0,
+bool SIInstrInfo::findCommutedOpIndices(const MCInstrDesc &Desc,
+                                        unsigned &SrcOpIdx0,
                                         unsigned &SrcOpIdx1) const {
   if (!Desc.isCommutable())
     return false;

diff  --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index a0ca2fa8a983e..025faec0e2ccf 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -268,7 +268,7 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
   bool findCommutedOpIndices(const MachineInstr &MI, unsigned &SrcOpIdx0,
                              unsigned &SrcOpIdx1) const override;
 
-  bool findCommutedOpIndices(MCInstrDesc Desc, unsigned &SrcOpIdx0,
+  bool findCommutedOpIndices(const MCInstrDesc &Desc, unsigned &SrcOpIdx0,
                              unsigned &SrcOpIdx1) const;
 
   bool isBranchOffsetInRange(unsigned BranchOpc,

diff  --git a/llvm/lib/Target/Lanai/LanaiDelaySlotFiller.cpp b/llvm/lib/Target/Lanai/LanaiDelaySlotFiller.cpp
index cafe93bf8f4b3..a10124c2d5c12 100644
--- a/llvm/lib/Target/Lanai/LanaiDelaySlotFiller.cpp
+++ b/llvm/lib/Target/Lanai/LanaiDelaySlotFiller.cpp
@@ -224,7 +224,7 @@ void Filler::insertDefsUses(MachineBasicBlock::instr_iterator MI,
                             SmallSet<unsigned, 32> &RegDefs,
                             SmallSet<unsigned, 32> &RegUses) {
   // If MI is a call or return, just examine the explicit non-variadic operands.
-  MCInstrDesc MCID = MI->getDesc();
+  const MCInstrDesc &MCID = MI->getDesc();
   unsigned E = MI->isCall() || MI->isReturn() ? MCID.getNumOperands()
                                               : MI->getNumOperands();
   for (unsigned I = 0; I != E; ++I) {

diff  --git a/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp b/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
index 6b99b283f49fb..a1e90cd104a97 100644
--- a/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
@@ -80,7 +80,7 @@ void SPIRVInstPrinter::printInst(const MCInst *MI, uint64_t Address,
     printOpExtInst(MI, OS);
   } else {
     // Print any extra operands for variadic instructions.
-    MCInstrDesc MCDesc = MII.get(OpCode);
+    const MCInstrDesc &MCDesc = MII.get(OpCode);
     if (MCDesc.isVariadic()) {
       const unsigned NumFixedOps = MCDesc.getNumOperands();
       const unsigned LastFixedIndex = NumFixedOps - 1;
@@ -185,7 +185,7 @@ void SPIRVInstPrinter::printInst(const MCInst *MI, uint64_t Address,
 void SPIRVInstPrinter::printOpExtInst(const MCInst *MI, raw_ostream &O) {
   // The fixed operands have already been printed, so just need to decide what
   // type of ExtInst operands to print based on the instruction set and number.
-  MCInstrDesc MCDesc = MII.get(MI->getOpcode());
+  const MCInstrDesc &MCDesc = MII.get(MI->getOpcode());
   unsigned NumFixedOps = MCDesc.getNumOperands();
   const auto NumOps = MI->getNumOperands();
   if (NumOps == NumFixedOps)
@@ -200,7 +200,7 @@ void SPIRVInstPrinter::printOpExtInst(const MCInst *MI, raw_ostream &O) {
 void SPIRVInstPrinter::printOpDecorate(const MCInst *MI, raw_ostream &O) {
   // The fixed operands have already been printed, so just need to decide what
   // type of decoration operands to print based on the Decoration type.
-  MCInstrDesc MCDesc = MII.get(MI->getOpcode());
+  const MCInstrDesc &MCDesc = MII.get(MI->getOpcode());
   unsigned NumFixedOps = MCDesc.getNumOperands();
 
   if (NumFixedOps != MI->getNumOperands()) {

diff  --git a/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVMCCodeEmitter.cpp b/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVMCCodeEmitter.cpp
index cd95ca63d4075..5555adc190101 100644
--- a/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVMCCodeEmitter.cpp
@@ -61,7 +61,7 @@ using EndianWriter = support::endian::Writer;
 // output register in operand 0. If so, we need to swap operands 0 and 1 so the
 // type comes first in the output, despide coming second in the MCInst.
 static bool hasType(const MCInst &MI, const MCInstrInfo &MII) {
-  MCInstrDesc MCDesc = MII.get(MI.getOpcode());
+  const MCInstrDesc &MCDesc = MII.get(MI.getOpcode());
   // If we define an output, and have at least one other argument.
   if (MCDesc.getNumDefs() == 1 && MCDesc.getNumOperands() >= 2) {
     // Check if we define an ID, and take a type as operand 1.

diff  --git a/llvm/unittests/CodeGen/LexicalScopesTest.cpp b/llvm/unittests/CodeGen/LexicalScopesTest.cpp
index a68db50808628..93ff4dc6a8b21 100644
--- a/llvm/unittests/CodeGen/LexicalScopesTest.cpp
+++ b/llvm/unittests/CodeGen/LexicalScopesTest.cpp
@@ -58,8 +58,8 @@ class LexicalScopesTest : public testing::Test {
   // Some meaningless instructions -- the first is fully meaningless,
   // while the second is supposed to impersonate DBG_VALUEs through its
   // opcode.
-  MCInstrDesc BeanInst;
-  MCInstrDesc DbgValueInst;
+  MCInstrDesc BeanInst{};
+  MCInstrDesc DbgValueInst{};
 
   LexicalScopesTest() : Ctx(), Mod("beehives", Ctx) {
     memset(&BeanInst, 0, sizeof(BeanInst));


        


More information about the llvm-commits mailing list