rengolin added a comment.

In http://reviews.llvm.org/D14121#276389, @t.p.northover wrote:

> If you're on Linux or something you need "clang -target x86_64-apple-darwin 
> -arch armv7 -c tmp.s".


x86_64 + ARMv7? This doesn't make sense... What is this trying to achieve?

> I suspect the reason for this hack is that we've already changed the triple 
> to "thumbv7-apple-iosN" by this point (because -arch armv7 compiles to 
> Thumb), so it needs undoing for .s files. It might be reasonably easy to push 
> the TY_PP_Asm check back into the Darwin codepath, or it might be horrible. 
> So much has changed since 2011.


I think so. If Darwin is special, in which it treats "armv7" as Thumb, then it 
needs special treatment, not the other way around.

Also, Alexandros, it would be good to add the same set of tests for Darwin, to 
make sure we're not messing up their side.

cheers,
--renato


================
Comment at: lib/Driver/ToolChain.cpp:472
@@ -471,2 +471,3 @@
+    bool ThumbDefault = (ARM::parseArchProfile(Suffix) == ARM::PK_M) ||
       (Suffix.startswith("v7") && getTriple().isOSBinFormatMachO());
     // FIXME: this is invalid for WindowsCE
----------------
labrinea wrote:
> rengolin wrote:
> > You could cache the profile and use it here, too.
> I don't see any checks based on profile in this line.
Sorry, not profile, but ARMTargetParser function.

You have just replaced:
    Suffix.startswith("v6m") || Suffix.startswith("v7m") || 
Suffix.startswith("v7em")
with:
    ARM::parseArchProfile(Suffix) == ARM::PK_M

You should also replace:
    Suffix.startswith("v7")
with:
    ARM::parseArchVersion(Suffix) == 7

================
Comment at: test/Driver/arm-ias-Wa.s:75
@@ +74,3 @@
+
+// RUN: %clang -target thumbv7m-none-eabi -c %s -### 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-M-PROFILE %s
----------------
labrinea wrote:
> rengolin wrote:
> > You should also add "armv7m" and check that it defaults to Thumb, no?
> It does default to thumb, if we are happy with this check I can add it.
That will give people looking at your commit in the future the idea of what you 
were trying to achieve.


http://reviews.llvm.org/D14121



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to