<p dir="ltr"><br>
On Oct 27, 2015 11:06, "Renato Golin" <<a href="mailto:renato.golin@linaro.org">renato.golin@linaro.org</a>> wrote:<br>
><br>
> rengolin added a comment.<br>
><br>
> In <a href="http://reviews.llvm.org/D14121#276389">http://reviews.llvm.org/D14121#276389</a>, @t.p.northover wrote:<br>
><br>
> > If you're on Linux or something you need "clang -target x86_64-apple-darwin -arch armv7 -c tmp.s".<br>
><br>
><br>
> x86_64 + ARMv7? This doesn't make sense... What is this trying to achieve?</p>
<p dir="ltr">It sets the underlying platform to a Darwin one so that -arch armv7 flag works as expected.</p>
<p dir="ltr">What I keep meaning to fix is that specifying " thumbv7-apple-ios" directly hits an assertion failure.</p>
<p dir="ltr">Either way, I think the patch as it is now is fine, but we'd need to be careful if we wanted to remove the TY_PP_Asm check.</p>
<p dir="ltr">Tim.<br>
><br>
> > 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.<br>
><br>
><br>
> I think so. If Darwin is special, in which it treats "armv7" as Thumb, then it needs special treatment, not the other way around.<br>
><br>
> 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.<br>
><br>
> cheers,<br>
> --renato<br>
><br>
><br>
> ================<br>
> Comment at: lib/Driver/ToolChain.cpp:472<br>
> @@ -471,2 +471,3 @@<br>
> +    bool ThumbDefault = (ARM::parseArchProfile(Suffix) == ARM::PK_M) ||<br>
>        (Suffix.startswith("v7") && getTriple().isOSBinFormatMachO());<br>
>      // FIXME: this is invalid for WindowsCE<br>
> ----------------<br>
> labrinea wrote:<br>
> > rengolin wrote:<br>
> > > You could cache the profile and use it here, too.<br>
> > I don't see any checks based on profile in this line.<br>
> Sorry, not profile, but ARMTargetParser function.<br>
><br>
> You have just replaced:<br>
>     Suffix.startswith("v6m") || Suffix.startswith("v7m") || Suffix.startswith("v7em")<br>
> with:<br>
>     ARM::parseArchProfile(Suffix) == ARM::PK_M<br>
><br>
> You should also replace:<br>
>     Suffix.startswith("v7")<br>
> with:<br>
>     ARM::parseArchVersion(Suffix) == 7<br>
><br>
> ================<br>
> Comment at: test/Driver/arm-ias-Wa.s:75<br>
> @@ +74,3 @@<br>
> +<br>
> +// RUN: %clang -target thumbv7m-none-eabi -c %s -### 2>&1 \<br>
> +// RUN:   | FileCheck -check-prefix=CHECK-M-PROFILE %s<br>
> ----------------<br>
> labrinea wrote:<br>
> > rengolin wrote:<br>
> > > You should also add "armv7m" and check that it defaults to Thumb, no?<br>
> > It does default to thumb, if we are happy with this check I can add it.<br>
> That will give people looking at your commit in the future the idea of what you were trying to achieve.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D14121">http://reviews.llvm.org/D14121</a><br>
><br>
><br>
><br>
</p>