[llvm] r282643 - [RegisterBankInfo] Uniquely generate OperandsMapping.

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 28 15:20:49 PDT 2016


Author: qcolombet
Date: Wed Sep 28 17:20:49 2016
New Revision: 282643

URL: http://llvm.org/viewvc/llvm-project?rev=282643&view=rev
Log:
[RegisterBankInfo] Uniquely generate OperandsMapping.

This is a step toward statically allocate InstructionMapping. Like the
previous few commits, the goal is to move toward a TableGen'ed like
structure with no dynamic allocation at all.

This should already improve compile time by getting rid of a bunch of
memmove of SmallVectors.

Modified:
    llvm/trunk/include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h
    llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp
    llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h?rev=282643&r1=282642&r2=282643&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/RegisterBankInfo.h Wed Sep 28 17:20:49 2016
@@ -184,12 +184,11 @@ public:
     /// Cost of this mapping.
     unsigned Cost;
     /// Mapping of all the operands.
-    /// Note: Use a SmallVector to avoid heap allocation in most cases.
-    SmallVector<const ValueMapping *, 8> OperandsMapping;
+    const ValueMapping *OperandsMapping;
     /// Number of operands.
     unsigned NumOperands;
 
-    const ValueMapping *&getOperandMapping(unsigned i) {
+    const ValueMapping &getOperandMapping(unsigned i) {
       assert(i < getNumOperands() && "Out of bound operand");
       return OperandsMapping[i];
     }
@@ -203,11 +202,13 @@ public:
     /// at the index i.
     ///
     /// \pre ID != InvalidMappingID
-    InstructionMapping(unsigned ID, unsigned Cost, unsigned NumOperands)
-        : ID(ID), Cost(Cost), NumOperands(NumOperands) {
+    InstructionMapping(unsigned ID, unsigned Cost,
+                       const ValueMapping *OperandsMapping,
+                       unsigned NumOperands)
+        : ID(ID), Cost(Cost), OperandsMapping(OperandsMapping),
+          NumOperands(NumOperands) {
       assert(getID() != InvalidMappingID &&
              "Use the default constructor for invalid mapping");
-      OperandsMapping.resize(getNumOperands(), nullptr);
     }
 
     /// Default constructor.
@@ -227,26 +228,23 @@ public:
     /// \pre The mapping for the ith operand has been set.
     /// \pre The ith operand is a register.
     const ValueMapping &getOperandMapping(unsigned i) const {
-      const ValueMapping *&ValMapping =
+      const ValueMapping &ValMapping =
           const_cast<InstructionMapping *>(this)->getOperandMapping(i);
-      assert(ValMapping && "Trying to get the mapping for a non-reg operand?");
-      return *ValMapping;
+      return ValMapping;
     }
 
-    /// Check if the value mapping of the ith operand has been set.
-    bool isOperandMappingSet(unsigned i) const {
-      return const_cast<InstructionMapping *>(this)->getOperandMapping(i) !=
-             nullptr;
-    }
-
-    /// Get the value mapping of the ith operand.
-    void setOperandMapping(unsigned i, const ValueMapping &ValMapping) {
-      getOperandMapping(i) = &ValMapping;
+    /// Set the mapping for all the operands.
+    /// In other words, OpdsMapping should hold at least getNumOperands
+    /// ValueMapping.
+    void setOperandsMapping(const ValueMapping *OpdsMapping) {
+      OperandsMapping = OpdsMapping;
     }
 
     /// Check whether this object is valid.
     /// This is a lightweight check for obvious wrong instance.
-    bool isValid() const { return getID() != InvalidMappingID; }
+    bool isValid() const {
+      return getID() != InvalidMappingID && OperandsMapping;
+    }
 
     /// Verifiy that this mapping makes sense for \p MI.
     /// \pre \p MI must be connected to a MachineFunction.
@@ -380,6 +378,10 @@ protected:
   /// This shouldn't be needed when everything gets TableGen'ed.
   mutable DenseMap<unsigned, const ValueMapping *> MapOfValueMappings;
 
+  /// Keep dynamically allocated array of ValueMapping in a separate map.
+  /// This shouldn't be needed when everything gets TableGen'ed.
+  mutable DenseMap<unsigned, ValueMapping *> MapOfOperandsMappings;
+
   /// Create a RegisterBankInfo that can accomodate up to \p NumRegBanks
   /// RegisterBank instances.
   ///
@@ -466,6 +468,39 @@ protected:
                                       unsigned NumBreakDowns) const;
   /// @}
 
+  /// Methods to get a uniquely generated array of ValueMapping.
+  /// @{
+
+  /// Get the uniquely generated array of ValueMapping for the
+  /// elements of between \p Begin and \p End.
+  ///
+  /// Elements that are nullptr will be replaced by
+  /// invalid ValueMapping (ValueMapping::isValid == false).
+  ///
+  /// \pre The pointers on ValueMapping between \p Begin and \p End
+  /// must uniquely identify a ValueMapping. Otherwise, there is no
+  /// guarantee that the return instance will be unique, i.e., another
+  /// OperandsMapping could have the same content.
+  template <typename Iterator>
+  const ValueMapping *getOperandsMapping(Iterator Begin, Iterator End) const;
+
+  /// Get the uniquely generated array of ValueMapping for the
+  /// elements of \p OpdsMapping.
+  ///
+  /// Elements of \p OpdsMapping that are nullptr will be replaced by
+  /// invalid ValueMapping (ValueMapping::isValid == false).
+  const ValueMapping *getOperandsMapping(
+      const SmallVectorImpl<const ValueMapping *> &OpdsMapping) const;
+
+  /// Get the uniquely generated array of ValueMapping for the
+  /// given arguments.
+  ///
+  /// Arguments that are nullptr will be replaced by invalid
+  /// ValueMapping (ValueMapping::isValid == false).
+  const ValueMapping *getOperandsMapping(
+      std::initializer_list<const ValueMapping *> OpdsMapping) const;
+  /// @}
+
   /// Get the register bank for the \p OpIdx-th operand of \p MI form
   /// the encoding constraints, if any.
   ///

Modified: llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp?rev=282643&r1=282642&r2=282643&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp Wed Sep 28 17:20:49 2016
@@ -41,6 +41,10 @@ STATISTIC(NumValueMappingsCreated,
           "Number of value mappings dynamically created");
 STATISTIC(NumValueMappingsAccessed,
           "Number of value mappings dynamically accessed");
+STATISTIC(NumOperandsMappingsCreated,
+          "Number of operands mappings dynamically created");
+STATISTIC(NumOperandsMappingsAccessed,
+          "Number of operands mappings dynamically accessed");
 
 const unsigned RegisterBankInfo::DefaultMappingID = UINT_MAX;
 const unsigned RegisterBankInfo::InvalidMappingID = UINT_MAX - 1;
@@ -233,6 +237,7 @@ const TargetRegisterClass *RegisterBankI
 RegisterBankInfo::InstructionMapping
 RegisterBankInfo::getInstrMappingImpl(const MachineInstr &MI) const {
   RegisterBankInfo::InstructionMapping Mapping(DefaultMappingID, /*Cost*/ 1,
+                                               /*OperandsMapping*/ nullptr,
                                                MI.getNumOperands());
   const MachineFunction &MF = *MI.getParent()->getParent();
   const TargetSubtargetInfo &STI = MF.getSubtarget();
@@ -251,7 +256,10 @@ RegisterBankInfo::getInstrMappingImpl(co
   const RegisterBank *RegBank = nullptr;
   // Remember the size of the register for reuse for copy-like instructions.
   unsigned RegSize = 0;
-  for (unsigned OpIdx = 0, End = MI.getNumOperands(); OpIdx != End; ++OpIdx) {
+
+  unsigned NumOperands = MI.getNumOperands();
+  SmallVector<const ValueMapping *, 8> OperandsMapping(NumOperands);
+  for (unsigned OpIdx = 0; OpIdx != NumOperands; ++OpIdx) {
     const MachineOperand &MO = MI.getOperand(OpIdx);
     if (!MO.isReg())
       continue;
@@ -289,11 +297,13 @@ RegisterBankInfo::getInstrMappingImpl(co
     }
     RegBank = CurRegBank;
     RegSize = getSizeInBits(Reg, MRI, TRI);
-    Mapping.setOperandMapping(OpIdx, getValueMapping(0, RegSize, *CurRegBank));
+    OperandsMapping[OpIdx] = &getValueMapping(0, RegSize, *CurRegBank);
   }
 
-  if (CompleteMapping)
+  if (CompleteMapping) {
+    Mapping.setOperandsMapping(getOperandsMapping(OperandsMapping));
     return Mapping;
+  }
 
   assert(isCopyLike && "We should have bailed on non-copies at this point");
   // For copy like instruction, if none of the operand has a register
@@ -304,18 +314,19 @@ RegisterBankInfo::getInstrMappingImpl(co
   // This is a copy-like instruction.
   // Propagate RegBank to all operands that do not have a
   // mapping yet.
-  for (unsigned OpIdx = 0, End = MI.getNumOperands(); OpIdx != End; ++OpIdx) {
+  for (unsigned OpIdx = 0; OpIdx != NumOperands; ++OpIdx) {
     const MachineOperand &MO = MI.getOperand(OpIdx);
     // Don't assign a mapping for non-reg operands.
     if (!MO.isReg())
       continue;
 
     // If a mapping already exists, do not touch it.
-    if (Mapping.isOperandMappingSet(OpIdx))
+    if (OperandsMapping[OpIdx])
       continue;
 
-    Mapping.setOperandMapping(OpIdx, getValueMapping(0, RegSize, *RegBank));
+    OperandsMapping[OpIdx] = &getValueMapping(0, RegSize, *RegBank);
   }
+  Mapping.setOperandsMapping(getOperandsMapping(OperandsMapping));
   return Mapping;
 }
 
@@ -383,6 +394,50 @@ RegisterBankInfo::getValueMapping(const
   return *ValMapping;
 }
 
+template <typename Iterator>
+const RegisterBankInfo::ValueMapping *
+RegisterBankInfo::getOperandsMapping(Iterator Begin, Iterator End) const {
+
+  ++NumOperandsMappingsAccessed;
+
+  // The addresses of the value mapping are unique.
+  // Therefore, we can use them directly to hash the operand mapping.
+  hash_code Hash = hash_combine_range(Begin, End);
+  const auto &It = MapOfOperandsMappings.find(Hash);
+  if (It != MapOfOperandsMappings.end())
+    return It->second;
+
+  ++NumOperandsMappingsCreated;
+
+  // Create the array of ValueMapping.
+  // Note: this array will not hash to this instance of operands
+  // mapping, because we use the pointer of the ValueMapping
+  // to hash and we expect them to uniquely identify an instance
+  // of value mapping.
+  ValueMapping *&Res = MapOfOperandsMappings[Hash];
+  Res = new ValueMapping[std::distance(Begin, End)];
+  unsigned Idx = 0;
+  for (Iterator It = Begin; It != End; ++It, ++Idx) {
+    const ValueMapping *ValMap = *It;
+    if (!ValMap)
+      continue;
+    Res[Idx] = *ValMap;
+  }
+  return Res;
+}
+
+const RegisterBankInfo::ValueMapping *RegisterBankInfo::getOperandsMapping(
+    const SmallVectorImpl<const RegisterBankInfo::ValueMapping *> &OpdsMapping)
+    const {
+  return getOperandsMapping(OpdsMapping.begin(), OpdsMapping.end());
+}
+
+const RegisterBankInfo::ValueMapping *RegisterBankInfo::getOperandsMapping(
+    std::initializer_list<const RegisterBankInfo::ValueMapping *> OpdsMapping)
+    const {
+  return getOperandsMapping(OpdsMapping.begin(), OpdsMapping.end());
+}
+
 RegisterBankInfo::InstructionMapping
 RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
     RegisterBankInfo::InstructionMapping Mapping = getInstrMappingImpl(MI);
@@ -546,14 +601,14 @@ bool RegisterBankInfo::InstructionMappin
   for (unsigned Idx = 0; Idx < NumOperands; ++Idx) {
     const MachineOperand &MO = MI.getOperand(Idx);
     if (!MO.isReg()) {
-      assert(!isOperandMappingSet(Idx) &&
+      assert(!getOperandMapping(Idx).isValid() &&
              "We should not care about non-reg mapping");
       continue;
     }
     unsigned Reg = MO.getReg();
     if (!Reg)
       continue;
-    assert(isOperandMappingSet(Idx) &&
+    assert(getOperandMapping(Idx).isValid() &&
            "We must have a mapping for reg operands");
     const RegisterBankInfo::ValueMapping &MOMapping = getOperandMapping(Idx);
     (void)MOMapping;

Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp?rev=282643&r1=282642&r2=282643&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp Wed Sep 28 17:20:49 2016
@@ -201,16 +201,24 @@ AArch64RegisterBankInfo::getInstrAlterna
     if (MI.getNumOperands() != 3)
       break;
     InstructionMappings AltMappings;
-    InstructionMapping GPRMapping(/*ID*/ 1, /*Cost*/ 1, /*NumOperands*/ 3);
-    InstructionMapping FPRMapping(/*ID*/ 2, /*Cost*/ 1, /*NumOperands*/ 3);
-    for (unsigned Idx = 0; Idx != 3; ++Idx) {
-      GPRMapping.setOperandMapping(
-          Idx, AArch64::ValMappings[AArch64::getRegBankBaseIdx(Size) +
-                                    AArch64::FirstGPR]);
-      FPRMapping.setOperandMapping(
-          Idx, AArch64::ValMappings[AArch64::getRegBankBaseIdx(Size) +
-                                    AArch64::FirstFPR]);
-    }
+    InstructionMapping GPRMapping(/*ID*/ 1, /*Cost*/ 1, nullptr,
+                                  /*NumOperands*/ 3);
+    InstructionMapping FPRMapping(/*ID*/ 2, /*Cost*/ 1, nullptr,
+                                  /*NumOperands*/ 3);
+    GPRMapping.setOperandsMapping(getOperandsMapping(
+        {&AArch64::ValMappings[AArch64::getRegBankBaseIdx(Size) +
+                               AArch64::FirstGPR],
+         &AArch64::ValMappings[AArch64::getRegBankBaseIdx(Size) +
+                               AArch64::FirstGPR],
+         &AArch64::ValMappings[AArch64::getRegBankBaseIdx(Size) +
+                               AArch64::FirstGPR]}));
+    FPRMapping.setOperandsMapping(getOperandsMapping(
+        {&AArch64::ValMappings[AArch64::getRegBankBaseIdx(Size) +
+                               AArch64::FirstFPR],
+         &AArch64::ValMappings[AArch64::getRegBankBaseIdx(Size) +
+                               AArch64::FirstFPR],
+         &AArch64::ValMappings[AArch64::getRegBankBaseIdx(Size) +
+                               AArch64::FirstFPR]}));
     AltMappings.emplace_back(std::move(GPRMapping));
     AltMappings.emplace_back(std::move(FPRMapping));
     return AltMappings;
@@ -266,13 +274,14 @@ AArch64RegisterBankInfo::getInstrMapping
       return Mapping;
   }
 
+  unsigned NumOperands = MI.getNumOperands();
   RegisterBankInfo::InstructionMapping Mapping =
-      InstructionMapping{DefaultMappingID, 1, MI.getNumOperands()};
+      InstructionMapping{DefaultMappingID, 1, nullptr, NumOperands};
 
   // Track the size and bank of each register.  We don't do partial mappings.
-  SmallVector<unsigned, 4> OpBaseIdx(MI.getNumOperands());
-  SmallVector<unsigned, 4> OpFinalIdx(MI.getNumOperands());
-  for (unsigned Idx = 0; Idx < MI.getNumOperands(); ++Idx) {
+  SmallVector<unsigned, 4> OpBaseIdx(NumOperands);
+  SmallVector<unsigned, 4> OpFinalIdx(NumOperands);
+  for (unsigned Idx = 0; Idx < NumOperands; ++Idx) {
     auto &MO = MI.getOperand(Idx);
     if (!MO.isReg())
       continue;
@@ -318,9 +327,11 @@ AArch64RegisterBankInfo::getInstrMapping
   }
 
   // Finally construct the computed mapping.
-  for (unsigned Idx = 0; Idx < MI.getNumOperands(); ++Idx)
+  SmallVector<const ValueMapping *, 8> OpdsMapping(NumOperands);
+  for (unsigned Idx = 0; Idx < NumOperands; ++Idx)
     if (MI.getOperand(Idx).isReg())
-      Mapping.setOperandMapping(Idx, AArch64::ValMappings[OpFinalIdx[Idx]]);
+      OpdsMapping[Idx] = &AArch64::ValMappings[OpFinalIdx[Idx]];
 
+  Mapping.setOperandsMapping(getOperandsMapping(OpdsMapping));
   return Mapping;
 }




More information about the llvm-commits mailing list