[PATCH] D14121: Thumb state not being passed through to LLVM triple when using clang -cc1as

[Renato Golin via cfe-commits]

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