[tablegen] Merge duplicate definitions of getMinimalTypeForRange. NFC.

[dsanders]

Summary

Depends on D25614

[dsanders]

ping

[dsanders]

ping

This is a pre-requisite of D25618 which has been LGTM'd

[qcolombet]

utils/TableGen/Types.cpp

18

Get rid of that if.

23

Adapt the assert to the MaxSize.
Also check that MaxSize is smaller or equal than 64.

utils/TableGen/Types.h

20

I would move a bit differently with the implementation.

I would say that MaxSize is just for static assert and that it must be smaller or equal to 64.

[dsanders]

utils/TableGen/Types.cpp

18

utils/TableGen/Types.h

20

The MaxSize argument is only there to avoid a functional change in this particular change. Two of the getMinimalTypeForRange() implementations (RegisterInfoEmitter.cpp and AsmWriterEmitter.cpp) didn't allow uint64_t while the third did allow it. I suspect the only reason the limitation was there is that the relevant enums haven't grown large enough to warrant uint64_t yet. I was thinking that a follow-up patch could lift this limitation and remove the MaxSize argument but I didn't want to make that functional change a prerequisite of D25618. If you prefer, then I can make all three callers allow uint64_t in this change and remove MaxSize.

[qcolombet]

utils/TableGen/Types.h

20

I know :).
What I am saying is that the MaxSize should be used in the assert, but not in the if.
I.e., I want straight line code for all the cases.

[dsanders]

utils/TableGen/Types.h

20

I think I see what you're saying now. You're looking for something like this:

assert((MaxSize == 64 ? Range <= 0xFFFFFFFFFFFFFFFFULL : Range <= 0xFFFFFFFFULL) && "Enum too large");
if (Range > 0xFFFFFFFFULL)
  return "uint64_t";
if (Range > 0xFFFF)
  return "uint32_t";
if (Range > 0xFF)
  return "uint16_t";
return "uint8_t";

[dsanders]

Only use MaxSize in an assert() and mark it unused to avoid issues with -Werror

[qcolombet]

LGTM

One nitpick.

Thanks Daniel!

utils/TableGen/Types.h

20

Yep, that's it.
Also add a MaxSize <= 64 assert.

[dsanders]

Thanks

utils/TableGen/Types.h

20

I've added a MaxSize <= 64 assert in the commit.

utils/TableGen/AsmMatcherEmitter.cpp

 //
 //   3. For a match success, each operand class that has a 'ParserMethod'
 //      becomes part of a switch from where the custom method is called.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CodeGenTarget.h"
 #include "SubtargetFeatureInfo.h"
+#include "Types.h"
 #include "llvm/ADT/CachedHashString.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
static void emitMatchRegisterAltName(CodeGenTarget &Target, Record *AsmParser,
   OS << "static unsigned MatchRegisterAltName(StringRef Name) {\n";
 
   StringMatcher("Name", Matches, OS).Emit();
 
   OS << "  return 0;\n";
   OS << "}\n\n";
 }
 
-static const char *getMinimalTypeForRange(uint64_t Range) {
-  assert(Range <= 0xFFFFFFFFFFFFFFFFULL && "Enum too large");
-  if (Range > 0xFFFFFFFFULL)
-    return "uint64_t";
-  if (Range > 0xFFFF)
-    return "uint32_t";
-  if (Range > 0xFF)
-    return "uint16_t";
-  return "uint8_t";
-}
-
 static const char *getMinimalRequiredFeaturesType(const AsmMatcherInfo &Info) {
   uint64_t MaxIndex = Info.SubtargetFeatures.size();
   if (MaxIndex > 0)
     MaxIndex--;
   return getMinimalTypeForRange(1ULL << MaxIndex);
 }
 
 /// emitSubtargetFeatureFlagEnumeration - Emit the subtarget feature flag

utils/TableGen/AsmWriterEmitter.cpp

 // This tablegen backend emits an assembly printer for the current target.
 // Note that this is currently fairly skeletal, but will grow over time.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AsmWriterInst.h"
 #include "CodeGenTarget.h"
 #include "SequenceToOffsetTable.h"
+#include "Types.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
while (!Instructions.empty())
       EmitInstructions(Instructions, O, PassSubtarget);
 
     O << "  }\n";
   }
 
   O << "}\n";
 }
 
-static const char *getMinimalTypeForRange(uint64_t Range) {
-  assert(Range < 0xFFFFFFFFULL && "Enum too large");
-  if (Range > 0xFFFF)
-    return "uint32_t";
-  if (Range > 0xFF)
-    return "uint16_t";
-  return "uint8_t";
-}
-
 static void
 emitRegisterNameString(raw_ostream &O, StringRef AltName,
                        const std::deque<CodeGenRegister> &Registers) {
   SequenceToOffsetTable<std::string> StringTable;
   SmallVector<std::string, 4> AsmNames(Registers.size());
   unsigned i = 0;
   for (const auto &Reg : Registers) {
     std::string &AsmName = AsmNames[i++];
for (const auto &Reg : Registers) {
     StringTable.add(AsmName);
   }
 
   StringTable.layout();
   O << "  static const char AsmStrs" << AltName << "[] = {\n";
   StringTable.emit(O, printChar);
   O << "  };\n\n";
 
-  O << "  static const " << getMinimalTypeForRange(StringTable.size()-1)
+  O << "  static const " << getMinimalTypeForRange(StringTable.size() - 1, 32)
     << " RegAsmOffset" << AltName << "[] = {";
   for (unsigned i = 0, e = Registers.size(); i != e; ++i) {
     if ((i % 14) == 0)
       O << "\n    ";
     O << StringTable.get(AsmNames[i]) << ", ";
   }
   O << "\n  };\n"
     << "\n";

utils/TableGen/CMakeLists.txt

add_tablegen(llvm-tblgen LLVM
   IntrinsicEmitter.cpp
   OptParserEmitter.cpp
   PseudoLoweringEmitter.cpp
   RegisterInfoEmitter.cpp
   SearchableTableEmitter.cpp
   SubtargetEmitter.cpp
   SubtargetFeatureInfo.cpp
   TableGen.cpp
+  Types.cpp
   X86DisassemblerTables.cpp
   X86ModRMFilters.cpp
   X86RecognizableInstr.cpp
   CTagsEmitter.cpp
   )

utils/TableGen/RegisterInfoEmitter.cpp

 // This tablegen backend is responsible for emitting a description of a target
 // register file for a code generator.  It uses instances of the Register,
 // RegisterAliases, and RegisterClass classes to gather this information.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CodeGenRegisters.h"
 #include "CodeGenTarget.h"
+#include "Types.h"
 #include "SequenceToOffsetTable.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/SparseBitVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Twine.h"
void RegisterInfoEmitter::runEnums(raw_ostream &OS,
   OS << "} // end namespace llvm\n\n";
   OS << "#endif // GET_REGINFO_ENUM\n\n";
 }
 
 static void printInt(raw_ostream &OS, int Val) {
   OS << Val;
 }
 
-static const char *getMinimalTypeForRange(uint64_t Range) {
-  assert(Range < 0xFFFFFFFFULL && "Enum too large");
-  if (Range > 0xFFFF)
-    return "uint32_t";
-  if (Range > 0xFF)
-    return "uint16_t";
-  return "uint8_t";
-}
-
 void RegisterInfoEmitter::
 EmitRegUnitPressure(raw_ostream &OS, const CodeGenRegBank &RegBank,
                     const std::string &ClassName) {
   unsigned NumRCs = RegBank.getRegClasses().size();
   unsigned NumSets = RegBank.getNumRegPressureSets();
 
   OS << "/// Get the weight in units of pressure for this register class.\n"
      << "const RegClassWeight &" << ClassName << "::\n"
EmitRegUnitPressure(raw_ostream &OS, const CodeGenRegBank &RegBank,
   }
   OS << "  };\n"
      << "  return PressureNameTable[Idx];\n"
      << "}\n\n";
 
   OS << "// Get the register unit pressure limit for this dimension.\n"
      << "// This limit must be adjusted dynamically for reserved registers.\n"
      << "unsigned " << ClassName << "::\n"
-     << "getRegPressureSetLimit(const MachineFunction &MF, unsigned Idx) const {\n"
-     << "  static const " << getMinimalTypeForRange(MaxRegUnitWeight)
+     << "getRegPressureSetLimit(const MachineFunction &MF, unsigned Idx) const "
+        "{\n"
+     << "  static const " << getMinimalTypeForRange(MaxRegUnitWeight, 32)
      << " PressureLimitTable[] = {\n";
   for (unsigned i = 0; i < NumSets; ++i ) {
     const RegUnitSet &RegUnits = RegBank.getRegSetAt(i);
     OS << "    " << RegUnits.Weight << ",  \t// " << i << ": "
        << RegUnits.Name << "\n";
   }
   OS << "  };\n"
      << "  return PressureLimitTable[Idx];\n"
EmitRegUnitPressure(raw_ostream &OS, const CodeGenRegBank &RegBank,
   PSetsSeqs.emit(OS, printInt, "-1");
   OS << "};\n\n";
 
   OS << "/// Get the dimensions of register pressure impacted by this "
      << "register class.\n"
      << "/// Returns a -1 terminated array of pressure set IDs\n"
      << "const int* " << ClassName << "::\n"
      << "getRegClassPressureSets(const TargetRegisterClass *RC) const {\n";
-  OS << "  static const " << getMinimalTypeForRange(PSetsSeqs.size()-1)
+  OS << "  static const " << getMinimalTypeForRange(PSetsSeqs.size() - 1, 32)
      << " RCSetStartTable[] = {\n    ";
   for (unsigned i = 0, e = NumRCs; i != e; ++i) {
     OS << PSetsSeqs.get(PSets[i]) << ",";
   }
   OS << "};\n"
      << "  return &RCSetsTable[RCSetStartTable[RC->getID()]];\n"
      << "}\n\n";
 
   OS << "/// Get the dimensions of register pressure impacted by this "
      << "register unit.\n"
      << "/// Returns a -1 terminated array of pressure set IDs\n"
      << "const int* " << ClassName << "::\n"
      << "getRegUnitPressureSets(unsigned RegUnit) const {\n"
      << "  assert(RegUnit < " << RegBank.getNumNativeRegUnits()
      << " && \"invalid register unit\");\n";
-  OS << "  static const " << getMinimalTypeForRange(PSetsSeqs.size()-1)
+  OS << "  static const " << getMinimalTypeForRange(PSetsSeqs.size() - 1, 32)
      << " RUSetStartTable[] = {\n    ";
   for (unsigned UnitIdx = 0, UnitEnd = RegBank.getNumNativeRegUnits();
        UnitIdx < UnitEnd; ++UnitIdx) {
     OS << PSetsSeqs.get(PSets[RegBank.getRegUnit(UnitIdx).RegClassUnitSetsIdx])
        << ",";
   }
   OS << "};\n"
      << "  return &RCSetsTable[RUSetStartTable[RegUnit]];\n"
if (Found == ~0u) {
       Rows.back().resize(SubRegIndicesSize);
       combine(&Idx, Rows.back());
     }
     RowMap.push_back(Found);
   }
 
   // Output the row map if there is multiple rows.
   if (Rows.size() > 1) {
-    OS << "  static const " << getMinimalTypeForRange(Rows.size()) << " RowMap["
-       << SubRegIndicesSize << "] = {\n    ";
+    OS << "  static const " << getMinimalTypeForRange(Rows.size(), 32)
+       << " RowMap[" << SubRegIndicesSize << "] = {\n    ";
     for (unsigned i = 0, e = SubRegIndicesSize; i != e; ++i)
       OS << RowMap[i] << ", ";
     OS << "\n  };\n";
   }
 
   // Output the rows.
-  OS << "  static const " << getMinimalTypeForRange(SubRegIndicesSize + 1)
+  OS << "  static const " << getMinimalTypeForRange(SubRegIndicesSize + 1, 32)
      << " Rows[" << Rows.size() << "][" << SubRegIndicesSize << "] = {\n";
   for (unsigned r = 0, re = Rows.size(); r != re; ++r) {
     OS << "    { ";
     for (unsigned i = 0, e = SubRegIndicesSize; i != e; ++i)
       if (Rows[r][i])
         OS << Rows[r][i]->EnumValue << ", ";
       else
         OS << "0, ";

utils/TableGen/Types.h

+//===- Types.h - Helper for the selection of C++ types. ---------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_UTILS_TABLEGEN_TYPES_H
+#define LLVM_UTILS_TABLEGEN_TYPES_H
+
+#include <cstdint>
+
+namespace llvm {
+/// Returns the smallest unsigned integer type that can hold the given range.
+/// MaxSize indicates the largest size of integer to consider (in bits) and only
+/// supports values of at least 32.
+const char *getMinimalTypeForRange(uint64_t Range, unsigned MaxSize = 64);
+}
+
+#endif

utils/TableGen/Types.cpp

+//===- Types.cpp - Helper for the selection of C++ data types. ------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Types.h"
+
+// For LLVM_ATTRIBUTE_UNUSED
+#include "llvm/Support/Compiler.h"
+
+#include <cassert>
+
+using namespace llvm;
+
+const char *llvm::getMinimalTypeForRange(uint64_t Range, unsigned MaxSize LLVM_ATTRIBUTE_UNUSED) {
+  // TODO: The original callers only used 32 and 64 so these are the only
+  //       values permitted. Rather than widen the supported values we should
+  //       allow 64 for the callers that currently use 32 and remove the
+  //       argument altogether.
+  assert((MaxSize == 32 || MaxSize == 64) && "Unexpected size");
+  assert(((MaxSize > 32) ? Range <= 0xFFFFFFFFFFFFFFFFULL
+                         : Range <= 0xFFFFFFFFULL) &&
+         "Enum too large");
+
+  if (Range > 0xFFFFFFFFULL)
+    return "uint64_t";
+  if (Range > 0xFFFF)
+    return "uint32_t";
+  if (Range > 0xFF)
+    return "uint16_t";
+  return "uint8_t";
+}