[PATCH] Driver: add target definition for Windows on ARM

[Saleem Abdulrasool]

================
Comment at: lib/Basic/Targets.cpp:4425-4426
@@ +4424,4 @@
+    // TODO base this on the actual triple
+    Builder.defineMacro("_M_ARM", "7");
+    // TODO map the complete set of values
+    // 31: VFPv3 40: VFPv4
----------------
Reid Kleckner wrote:
> Any reason not to just do that?
I couldnt figure out how to get MSVC to provide the values for _M_ARM_FP that it supports.

================
Comment at: lib/Basic/Targets.cpp:4437
@@ +4436,3 @@
+    : WindowsARMTargetInfo(Triple) {
+    TheCXXABI.set(TargetCXXABI::GenericItanium);
+  }
----------------
Reid Kleckner wrote:
> Any reason not to use GenericARM?  GenericItanium uses a bit to implement member pointers that conflicts with the thumb bit of the PC.
Ah, I thought GenericARM was the ARM C++ ABI rather than IA-64 on ARM, fine by me to switch to that.

================
Comment at: lib/Basic/Targets.cpp:4450
@@ +4449,3 @@
+// Windows ARM, MS (C++) ABI
+class VisualStudioWindowsARMleTargetInfo : public WindowsARMTargetInfo {
+public:
----------------
Reid Kleckner wrote:
> This should probably be MicrosoftARMleTargetInfo for consistency.  Visual Studio is the IDE, not the C/C++ toolchain, which is usually called Microsoft Visual C++ (MSVC).
WFM, Ill change that.

================
Comment at: lib/Basic/Targets.cpp:4415
@@ +4414,3 @@
+    Builder.defineMacro("__cdecl", "");
+    Builder.defineMacro("__stdcall", "");
+    // The __inline keyword is similar to GNU inline.
----------------
David Majnemer wrote:
> This looks wrong.  You want to implement `checkCallingConvention`.
So I do, thanks!

================
Comment at: lib/Basic/Targets.cpp:4416
@@ +4415,3 @@
+    Builder.defineMacro("__stdcall", "");
+    // The __inline keyword is similar to GNU inline.
+    // http://msdn.microsoft.com/en-us/library/z8y1yy88(v=vs.71).aspx
----------------
Reid Kleckner wrote:
> I'm pretty sure this is wrong.  MSVC's __inline semantics are identical to C++ inline, with an extra quirk for extern __inline, which forces code emission.  Clang handles this in ASTContext::GetGVALinkageForFunction() under -fms-compatibility.  David is working on a patch for 'extern __inline' at the moment.
> 
> Perhaps we should change the way this works so that we treat '__inline' as C++ inline in C mode and 'inline' as C99 inline, and the __inline spelling would be enabled by -fms-extensions, which I assume you're using.
I think that should be controlled by -fms-compatibility no?  -fms-extensions is enabling extensions a la /Ze right?  (BTW, on a separate but related topic, perhaps -fms-compatibility should imply -fms-extensions since /Ze is default?).

That said, I can double check if __inline can be handled with the work that David did to support it.  If it can, then this can be entirely removed, which is even better!

================
Comment at: lib/Basic/Targets.cpp:3719
@@ +3718,3 @@
+      T.getArchName().substr(Offset).getAsInteger(10, Version);
+      assert(Version >= 7);
+      return true;
----------------
Renato Golin wrote:
> Reid Kleckner wrote:
> > Renato Golin wrote:
> > > Saleem Abdulrasool wrote:
> > > > Renato Golin wrote:
> > > > > Isn't an assert a bit too drastic? Maybe we should catch this somewhere else with an error message?
> > > > It is drastic, but AFAIK, WoA requires ARMv7 or greater.  Reporting an error message is certainly much nicer.
> > > Right. This being Clang, it should be relatively simple to bail on error (check the driver for details).
> > I see two easy ways to diagnose this:
> > 1. Diagnose in the Driver or Frontend that the triple armvN-windows is unsupported
> > 2. Call CodeGenModule::ErrorUnsupported() during IRGen when attempting to produce a libcall.  We don't generally like to emit diagnostics from IRGen, but for this kind of fringe thing it can be OK.
> I'm not sure we do any kind of validation at the triple level, so I think it should be fine to just call unsupported on the code gen level.
I think I prefer the first suggestion.  If there are no objections, I would prefer to use that.


http://llvm-reviews.chandlerc.com/D3241