[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 llvm-commits
mailing list