Hello LLVM folks (and Clang folks sense triple impacts the Clang driver
greatly)!

This patch removes the lazy-parsing from the Triple class. From what I
could tell, no one deeply relied on this, and it seems unlikely that we
want people to be creating enough triples to need them to parse lazily...

The code gets significantly simpler with this change, and I'm looking at
further changes that will make it more efficient and still cleaner.

In passing, it fixes a bug in 'hasEnvironment' which would return the wrong
answer if called before any other accessors on a triple. This category of
bugs is one of the primary reasons I'm not a fan of deferred parsing here.

All existing tests seem happy, and I can add a few more, as well as look
for any cleanups I missed, but I wanted to get some feedback on the general
direction.
diff --git a/include/llvm/ADT/Triple.h b/include/llvm/ADT/Triple.h
index a4b496e..1266da9 100644
--- a/include/llvm/ADT/Triple.h
+++ b/include/llvm/ADT/Triple.h
@@ -64,9 +64,7 @@ public:
     ptx32,   // PTX: ptx (32-bit)
     ptx64,   // PTX: ptx (64-bit)
     le32,    // le32: generic little-endian 32-bit CPU (PNaCl / Emscripten)
-    amdil,   // amdil: amd IL
-
-    InvalidArch
+    amdil   // amdil: amd IL
   };
   enum VendorType {
     UnknownVendor,
@@ -113,31 +111,29 @@ public:
 private:
   std::string Data;
 
-  /// The parsed arch type (or InvalidArch if uninitialized).
-  mutable ArchType Arch;
+  /// The parsed arch type.
+  ArchType Arch;
 
   /// The parsed vendor type.
-  mutable VendorType Vendor;
+  VendorType Vendor;
 
   /// The parsed OS type.
-  mutable OSType OS;
+  OSType OS;
 
   /// The parsed Environment type.
-  mutable EnvironmentType Environment;
+  EnvironmentType Environment;
 
-  bool isInitialized() const { return Arch != InvalidArch; }
   static ArchType ParseArch(StringRef ArchName);
   static VendorType ParseVendor(StringRef VendorName);
   static OSType ParseOS(StringRef OSName);
   static EnvironmentType ParseEnvironment(StringRef EnvironmentName);
-  void Parse() const;
 
 public:
   /// @name Constructors
   /// @{
 
   /// \brief Default constructor produces an empty, invalid triple.
-  Triple() : Data(), Arch(InvalidArch) {}
+  Triple() : Data(), Arch(), Vendor(), OS(), Environment() {}
 
   explicit Triple(const Twine &Str);
   Triple(const Twine &ArchStr, const Twine &VendorStr, const Twine &OSStr);
@@ -159,22 +155,13 @@ public:
   /// @{
 
   /// getArch - Get the parsed architecture type of this triple.
-  ArchType getArch() const {
-    if (!isInitialized()) Parse();
-    return Arch;
-  }
+  ArchType getArch() const { return Arch; }
 
   /// getVendor - Get the parsed vendor type of this triple.
-  VendorType getVendor() const {
-    if (!isInitialized()) Parse();
-    return Vendor;
-  }
+  VendorType getVendor() const { return Vendor; }
 
   /// getOS - Get the parsed operating system type of this triple.
-  OSType getOS() const {
-    if (!isInitialized()) Parse();
-    return OS;
-  }
+  OSType getOS() const { return OS; }
 
   /// hasEnvironment - Does this triple have the optional environment
   /// (fourth) component?
@@ -183,10 +170,7 @@ public:
   }
 
   /// getEnvironment - Get the parsed environment type of this triple.
-  EnvironmentType getEnvironment() const {
-    if (!isInitialized()) Parse();
-    return Environment;
-  }
+  EnvironmentType getEnvironment() const { return Environment; }
 
   /// getOSVersion - Parse the version number from the OS name component of the
   /// triple, if present.
diff --git a/include/llvm/Support/TargetRegistry.h b/include/llvm/Support/TargetRegistry.h
index ea55c91..aef43b1 100644
--- a/include/llvm/Support/TargetRegistry.h
+++ b/include/llvm/Support/TargetRegistry.h
@@ -786,7 +786,7 @@ namespace llvm {
   /// extern "C" void LLVMInitializeFooTargetInfo() {
   ///   RegisterTarget<Triple::foo> X(TheFooTarget, "foo", "Foo description");
   /// }
-  template<Triple::ArchType TargetArchType = Triple::InvalidArch,
+  template<Triple::ArchType TargetArchType = Triple::UnknownArch,
            bool HasJIT = false>
   struct RegisterTarget {
     RegisterTarget(Target &T, const char *Name, const char *Desc) {
diff --git a/lib/MC/MCDisassembler/EDDisassembler.cpp b/lib/MC/MCDisassembler/EDDisassembler.cpp
index ea77902..35cd3d7 100644
--- a/lib/MC/MCDisassembler/EDDisassembler.cpp
+++ b/lib/MC/MCDisassembler/EDDisassembler.cpp
@@ -47,8 +47,7 @@ static struct TripleMap triplemap[] = {
   { Triple::x86,          "i386-unknown-unknown"    },
   { Triple::x86_64,       "x86_64-unknown-unknown"  },
   { Triple::arm,          "arm-unknown-unknown"     },
-  { Triple::thumb,        "thumb-unknown-unknown"   },
-  { Triple::InvalidArch,  NULL,                     }
+  { Triple::thumb,        "thumb-unknown-unknown"   }
 };
 
 /// infoFromArch - Returns the TripleMap corresponding to a given architecture,
@@ -128,8 +127,6 @@ EDDisassembler::EDDisassembler(CPUKey &key) :
   ErrorStream(nulls()), 
   Key(key),
   TgtTriple(key.Triple.c_str()) {        
-  if (TgtTriple.getArch() == Triple::InvalidArch)
-    return;
   
   LLVMSyntaxVariant = getLLVMSyntaxVariant(TgtTriple.getArch(), key.Syntax);
   
diff --git a/lib/Support/Triple.cpp b/lib/Support/Triple.cpp
index 24ead35..af951fe 100644
--- a/lib/Support/Triple.cpp
+++ b/lib/Support/Triple.cpp
@@ -17,7 +17,6 @@ using namespace llvm;
 
 const char *Triple::getArchTypeName(ArchType Kind) {
   switch (Kind) {
-  case InvalidArch: return "<invalid>";
   case UnknownArch: return "unknown";
 
   case arm:     return "arm";
@@ -291,24 +290,19 @@ Triple::EnvironmentType Triple::ParseEnvironment(StringRef EnvironmentName) {
     .Default(UnknownEnvironment);
 }
 
-void Triple::Parse() const {
-  assert(!isInitialized() && "Invalid parse call.");
-
-  Arch = ParseArch(getArchName());
-  Vendor = ParseVendor(getVendorName());
-  OS = ParseOS(getOSName());
-  Environment = ParseEnvironment(getEnvironmentName());
-
-  assert(isInitialized() && "Failed to initialize!");
-}
-
 /// \brief Construct a triple from the string representation provided.
 ///
 /// This doesn't actually parse the string representation eagerly. Instead it
 /// stores it, and tracks the fact that it hasn't been parsed. The first time
 /// any of the structural queries are made, the string is parsed and the
 /// results cached in various members.
-Triple::Triple(const Twine &Str) : Data(Str.str()), Arch(InvalidArch) {}
+Triple::Triple(const Twine &Str)
+    : Data(Str.str()),
+      Arch(ParseArch(getArchName())),
+      Vendor(ParseVendor(getVendorName())),
+      OS(ParseOS(getOSName())),
+      Environment(ParseEnvironment(getEnvironmentName())) {
+}
 
 /// \brief Construct a triple from string representations of the architecture,
 /// vendor, and OS.
@@ -318,7 +312,10 @@ Triple::Triple(const Twine &Str) : Data(Str.str()), Arch(InvalidArch) {}
 /// string, and lazily parses it on use.
 Triple::Triple(const Twine &ArchStr, const Twine &VendorStr, const Twine &OSStr)
     : Data((ArchStr + Twine('-') + VendorStr + Twine('-') + OSStr).str()),
-      Arch(InvalidArch) {
+      Arch(ParseArch(ArchStr.str())),
+      Vendor(ParseVendor(VendorStr.str())),
+      OS(ParseOS(OSStr.str())),
+      Environment() {
 }
 
 /// \brief Construct a triple from string representations of the architecture,
@@ -331,7 +328,10 @@ Triple::Triple(const Twine &ArchStr, const Twine &VendorStr, const Twine &OSStr,
                const Twine &EnvironmentStr)
     : Data((ArchStr + Twine('-') + VendorStr + Twine('-') + OSStr + Twine('-') +
             EnvironmentStr).str()),
-      Arch(InvalidArch) {
+      Arch(ParseArch(ArchStr.str())),
+      Vendor(ParseVendor(VendorStr.str())),
+      OS(ParseOS(OSStr.str())),
+      Environment(ParseEnvironment(EnvironmentStr.str())) {
 }
 
 std::string Triple::normalize(StringRef Str) {
@@ -577,8 +577,7 @@ bool Triple::getMacOSXVersion(unsigned &Major, unsigned &Minor,
 }
 
 void Triple::setTriple(const Twine &Str) {
-  Data = Str.str();
-  Arch = InvalidArch;
+  *this = Triple(Str);
 }
 
 void Triple::setArch(ArchType Kind) {
@@ -632,7 +631,6 @@ void Triple::setOSAndEnvironmentName(StringRef Str) {
 static unsigned getArchPointerBitWidth(llvm::Triple::ArchType Arch) {
   switch (Arch) {
   case llvm::Triple::UnknownArch:
-  case llvm::Triple::InvalidArch:
     return 0;
 
   case llvm::Triple::msp430:
@@ -682,7 +680,6 @@ Triple Triple::get32BitArchVariant() const {
   Triple T(*this);
   switch (getArch()) {
   case Triple::UnknownArch:
-  case Triple::InvalidArch:
   case Triple::msp430:
     T.setArch(UnknownArch);
     break;
@@ -718,7 +715,6 @@ Triple Triple::get32BitArchVariant() const {
 Triple Triple::get64BitArchVariant() const {
   Triple T(*this);
   switch (getArch()) {
-  case Triple::InvalidArch:
   case Triple::UnknownArch:
   case Triple::amdil:
   case Triple::arm:
diff --git a/unittests/ADT/TripleTest.cpp b/unittests/ADT/TripleTest.cpp
index b95107d..5fe9afd 100644
--- a/unittests/ADT/TripleTest.cpp
+++ b/unittests/ADT/TripleTest.cpp
@@ -154,7 +154,7 @@ TEST(TripleTest, Normalization) {
   // Check that normalizing a permutated set of valid components returns a
   // triple with the unpermuted components.
   StringRef C[4];
-  for (int Arch = 1+Triple::UnknownArch; Arch < Triple::InvalidArch; ++Arch) {
+  for (int Arch = 1+Triple::UnknownArch; Arch <= Triple::amdil; ++Arch) {
     C[0] = Triple::getArchTypeName(Triple::ArchType(Arch));
     for (int Vendor = 1+Triple::UnknownVendor; Vendor <= Triple::PC;
          ++Vendor) {
@@ -273,11 +273,6 @@ TEST(TripleTest, BitWidthPredicates) {
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
 
-  T.setArch(Triple::InvalidArch);
-  EXPECT_FALSE(T.isArch16Bit());
-  EXPECT_FALSE(T.isArch32Bit());
-  EXPECT_FALSE(T.isArch64Bit());
-
   T.setArch(Triple::arm);
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to