On Oct 27, 2015 11:06, "Renato Golin" <renato.go...@linaro.org> wrote:
>
> 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?
It sets the underlying platform to a Darwin one so that -arch armv7 flag
works as expected.
What I keep meaning to fix is that specifying " thumbv7-apple-ios" directly
hits an assertion failure.
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.
Tim.
>
> > 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
