This revision was automatically updated to reflect the committed changes.
Closed by commit rC344367: [Driver] Add defaults for Android ARM FPUs.
(authored by danalbert, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53121?vs=169291&id=169450#toc
Repository:
rC Clang
http
peter.smith accepted this revision.
peter.smith added a comment.
This revision is now accepted and ready to land.
Looks good to me. By generic target I just meant
--target=arm-linux-androideabi21 with a -march to specify the revision, which
you've got in the test.
Repository:
rC Clang
https
kristof.beyls added inline comments.
Comment at: test/Driver/arm-mfpu.c:410
+
+// RUN: %clang -target armv7-linux-androideabi23 %s -mfpu=vfp3-d16 -### -c
2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-ARM-ANDROID-M-FP-D16 %s
danalbert wrote:
> >>! In D53121#
danalbert updated this revision to Diff 169291.
danalbert added a comment.
Addressed review comments.
Repository:
rC Clang
https://reviews.llvm.org/D53121
Files:
lib/Driver/ToolChains/Arch/ARM.cpp
test/Driver/arm-mfpu.c
Index: test/Driver/arm-mfpu.c
danalbert added inline comments.
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:360
+ unsigned ArchVersion;
+ if (ArchName.empty())
peter.smith wrote:
> Do you need to parse the arch version here? I would expect the -march=armv7
> to be reflected in getARMSu
peter.smith added a comment.
It looks like we might be able to simplify a bit, and I think there probably
could be some more test cases but no objections from me.
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:360
+ unsigned ArchVersion;
+ if (ArchName.empty())
---
kristof.beyls added a subscriber: peter.smith.
kristof.beyls added a comment.
In https://reviews.llvm.org/D53121#1261408, @srhines wrote:
> This LGTM, but we should wait to hear from Kristof before submitting.
Seems fine to me too. I'd maybe just add an additional test case to verify that
thin
srhines added a comment.
This LGTM, but we should wait to hear from Kristof before submitting.
Repository:
rC Clang
https://reviews.llvm.org/D53121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
danalbert added a comment.
Related to this but something I was less sure we should do: Android no longer
supports ARMv5. Should we make `arm-linux-androideabi` targets auto pull up to
armv7 if there's no `-march` flag?
Repository:
rC Clang
https://reviews.llvm.org/D53121
danalbert created this revision.
danalbert added reviewers: srhines, pirama.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.
Android mandates that devices have at least vfpv3-d16 until
Marshmallow and NEON after that. Still honor the user's decision, but
raise
10 matches
Mail list logo
