[llvm-commits] [llvm] r151024 - in /llvm/trunk: include/llvm/ADT/Triple.h include/llvm/Support/TargetRegistry.h lib/MC/MCDisassembler/EDDisassembler.cpp lib/Support/Triple.cpp unittests/ADT/TripleTest.cpp
Matt Beaumont-Gay
matthewbg at google.com
Mon Feb 20 19:57:40 PST 2012
Just a couple of nits.
On Mon, Feb 20, 2012 at 19:39, Chandler Carruth <chandlerc at gmail.com> wrote:
> Author: chandlerc
> Date: Mon Feb 20 21:39:36 2012
> New Revision: 151024
>
> URL: http://llvm.org/viewvc/llvm-project?rev=151024&view=rev
> Log:
> Switch the llvm::Triple class to immediately parse the triple string on
> construction. Simplify its interface, implementation, and users
> accordingly as there is no longer an 'uninitialized' state to check for.
> Also, fixes a bug lurking in the interface as there was one method that
> didn't correctly check for initialization.
>
> Modified:
> llvm/trunk/include/llvm/ADT/Triple.h
> llvm/trunk/include/llvm/Support/TargetRegistry.h
> llvm/trunk/lib/MC/MCDisassembler/EDDisassembler.cpp
> llvm/trunk/lib/Support/Triple.cpp
> llvm/trunk/unittests/ADT/TripleTest.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/Triple.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Triple.h?rev=151024&r1=151023&r2=151024&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/Triple.h (original)
> +++ llvm/trunk/include/llvm/ADT/Triple.h Mon Feb 20 21:39:36 2012
> @@ -64,9 +64,7 @@
> 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 @@
> 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.
Maybe update this comment? All fields are unknown, not invalid.
> - 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 @@
> /// @{
>
> /// 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 @@
> }
>
> /// 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.
>
> Modified: llvm/trunk/include/llvm/Support/TargetRegistry.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/TargetRegistry.h?rev=151024&r1=151023&r2=151024&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/TargetRegistry.h (original)
> +++ llvm/trunk/include/llvm/Support/TargetRegistry.h Mon Feb 20 21:39:36 2012
> @@ -786,7 +786,7 @@
> /// 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) {
>
> Modified: llvm/trunk/lib/MC/MCDisassembler/EDDisassembler.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCDisassembler/EDDisassembler.cpp?rev=151024&r1=151023&r2=151024&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCDisassembler/EDDisassembler.cpp (original)
> +++ llvm/trunk/lib/MC/MCDisassembler/EDDisassembler.cpp Mon Feb 20 21:39:36 2012
> @@ -47,8 +47,7 @@
> { 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 @@
> ErrorStream(nulls()),
> Key(key),
> TgtTriple(key.Triple.c_str()) {
> - if (TgtTriple.getArch() == Triple::InvalidArch)
> - return;
>
> LLVMSyntaxVariant = getLLVMSyntaxVariant(TgtTriple.getArch(), key.Syntax);
>
>
> Modified: llvm/trunk/lib/Support/Triple.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Triple.cpp?rev=151024&r1=151023&r2=151024&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/Triple.cpp (original)
> +++ llvm/trunk/lib/Support/Triple.cpp Mon Feb 20 21:39:36 2012
> @@ -17,7 +17,6 @@
>
> const char *Triple::getArchTypeName(ArchType Kind) {
> switch (Kind) {
> - case InvalidArch: return "<invalid>";
> case UnknownArch: return "unknown";
>
> case arm: return "arm";
> @@ -291,24 +290,19 @@
> .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.
Stale comment.
> -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 @@
> /// 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 @@
> 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 @@
> }
>
> void Triple::setTriple(const Twine &Str) {
> - Data = Str.str();
> - Arch = InvalidArch;
> + *this = Triple(Str);
> }
>
> void Triple::setArch(ArchType Kind) {
> @@ -632,7 +631,6 @@
> 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 T(*this);
> switch (getArch()) {
> case Triple::UnknownArch:
> - case Triple::InvalidArch:
> case Triple::msp430:
> T.setArch(UnknownArch);
> break;
> @@ -718,7 +715,6 @@
> Triple Triple::get64BitArchVariant() const {
> Triple T(*this);
> switch (getArch()) {
> - case Triple::InvalidArch:
> case Triple::UnknownArch:
> case Triple::amdil:
> case Triple::arm:
>
> Modified: llvm/trunk/unittests/ADT/TripleTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/TripleTest.cpp?rev=151024&r1=151023&r2=151024&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/TripleTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/TripleTest.cpp Mon Feb 20 21:39:36 2012
> @@ -154,7 +154,7 @@
> // 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) {
The loop condition here seems brittle. Not sure how to make it more
robust, though.
More information about the llvm-commits
mailing list