danalbert marked 3 inline comments as done.
danalbert added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:389
+ std::string DefaultFPU = getDefaultFPUName(Triple);
+ if (DefaultFPU != "") {
+ if (!llvm::ARM::getFPUFeatures(llvm::ARM::parseFPU(DefaultFPU),
Features))
----------------
peter.smith wrote:
> danalbert wrote:
> > peter.smith wrote:
> > > I'm wondering whether you need this bit of code anymore? In D53121 there
> > > needed to be a switch between vfpv3-d16 and neon based on Android
> > > version. With --target=armv7a-linux-android or --target=arm-linux-android
> > > -march=armv7a or any v7a -mcpu applicable to Android then you'll get
> > > feature Neon by default and won't need to do this? We could then move
> > > getDefaultFPUName out of ARM.cpp
> > I don't understand. This bit of code is the piece that provides the NEON
> > default. If I remove this then Android ARMv7 targets revert to no FPU
> > features.
> My understanding is that adding the equivalent of -fpu=neon here has the
> effect of adding extra feature flags to the call to clang -cc1. I can observe
> the difference with -###. However when compiling these are then added back in
> by TargetInfo::CreateTargetInfo() by calls like initFeatureMap that take the
> default features from the target. When invoking -cc1as the
> createMCSubtargetInfo via a long chain of function calls will add the
> FeatureNEON bit for armv7-a unless the -neon feature has been cleared.
>
> If I have it right I think the lack of floating point features in the clang
> -cc1 command line is not a problem as the defaults for armv7-a will put them
> in anyway unless they've been explicitly turned off via something like
> -mfloat-abi=soft. My information is based on reverse engineering the code so
> I could easily be missing something important though?
Oh, I thought that the cc1 args were the full story. I'll need to do some more
digging then to make sure that the behavior actually does match.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:251
case llvm::Triple::Android:
ABI = (SubArch == 7) ? FloatABI::SoftFP : FloatABI::Soft;
break;
----------------
peter.smith wrote:
> Not strictly related to this patch, but you'll probably want to make that
> (SubArch >= 7), I tried --target=armv8a-linux-android as an experiment and
> got my floating point support removed.
I actually uploaded a fix for that yesterday: https://reviews.llvm.org/D58477
Didn't submit because I wanted to wait until I knew I'd be around to babysit
build bots just in case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58314/new/
https://reviews.llvm.org/D58314
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
