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

Renato Golin renato.golin at linaro.org
Wed Apr 2 10:29:24 PDT 2014



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

================
Comment at: lib/Basic/Targets.cpp:4428
@@ +4427,3 @@
+    // 31: VFPv3 40: VFPv4
+    Builder.defineMacro("_M_ARM_FP", "31");
+  }
----------------
Saleem Abdulrasool wrote:
> Renato Golin wrote:
> > You default to cortex-a9 and VFPv4 at the same time, but A9 is VFPv3. You should either default to v3 or require A15/A7+.
> I might be mistaken (and if I am some how defaulting to VFPv4, then that is a bug).  I compacted the comment above, it really is saying:
> 
> 31 = VFPv3
> 40 = VFPv4
> 
> I did miss a FIXME to indicate that the value should be based upon the actual value of the VFP that is being specified.
Oh, right! Silly me...


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



More information about the cfe-commits mailing list