[PATCH] Canonicalise Windows target triple spellings

Saleem Abdulrasool abdulras at fb.com
Tue Mar 4 15:36:35 PST 2014


On Mar 4, 2014, at 1:56 PM, Reid Kleckner <rnk at google.com> wrote:

> 
>  This seems like the right direction, but we need to stage this so that we don't break Clang.
> 

Hah!  I realized that a bit too late :-).  Im working on the clang side of the change now.  But, regardless, you are right, we need to figure out how to stage this so that we do not break clang during that transition.

> 
> ================
> Comment at: include/llvm/ADT/Triple.h:334
> @@ -329,2 +333,3 @@
>   bool isOSCygMing() const {
> -    return getOS() == Triple::Cygwin || getOS() == Triple::MinGW32;
> +    return getOS() == Triple::Cygwin || getOS() == Triple::MinGW32 ||
> +           (getOS() == Triple::Win32 && (getEnvironment() == Triple::Cygnus ||
> ----------------
> Are you planning to remove the MinGW32 OS enum?

Yes, I would like to rename Win32 to Windows and remove MinGW32.  However, Id like to do that in a subsequent pass.  I don’t have a strong opinion on that, and if you think that it is better done in one shot, Im happy to merge that into this change.

> ================
> Comment at: lib/Support/Triple.cpp:531-539
> @@ +530,11 @@
> +      Components[3] = "msvc";
> +  } else if (OS == Triple::MinGW32) {
> +    Components.resize(4);
> +    Components[2] = "windows";
> +    Components[3] = (Format == Triple::ELF) ? "gnuelf" : "gnu";
> +  } else if (OS == Triple::Cygwin) {
> +    Components.resize(4);
> +    Components[2] = "windows";
> +    Components[3] = "cygnus";
> +  }
> +
> ----------------
> These canonicalizations will change the behavior of Clang, because it often looks for getOS() == llvm::Triple::MinGW32 / Cygwin in many places.  Can we defer this canonicalization until we've updated clang to do the right thing with the new triple?
> 
> 
> http://llvm-reviews.chandlerc.com/D2947

-- 
Saleem Abdulrasool
abdulras (at) fb (dot) com









More information about the llvm-commits mailing list