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

Reid Kleckner rnk at google.com
Wed Apr 2 13:56:02 PDT 2014



================
Comment at: lib/Basic/Targets.cpp:3719
@@ +3718,3 @@
+      T.getArchName().substr(Offset).getAsInteger(10, Version);
+      assert(Version >= 7);
+      return true;
----------------
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.

================
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
----------------
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.

================
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
----------------
Any reason not to just do that?

================
Comment at: lib/Basic/Targets.cpp:4437
@@ +4436,3 @@
+    : WindowsARMTargetInfo(Triple) {
+    TheCXXABI.set(TargetCXXABI::GenericItanium);
+  }
----------------
Any reason not to use GenericARM?  GenericItanium uses a bit to implement member pointers that conflicts with the thumb bit of the PC.

================
Comment at: lib/Basic/Targets.cpp:4450
@@ +4449,3 @@
+// Windows ARM, MS (C++) ABI
+class VisualStudioWindowsARMleTargetInfo : public WindowsARMTargetInfo {
+public:
----------------
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).


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



More information about the cfe-commits mailing list