[PATCH] D53121: [Driver] Add defaults for Android ARM FPUs.

[Peter Smith via Phabricator via cfe-commits]

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())
----------------
Do you need to parse the arch version here? I would expect the -march=armv7 to be reflected in getARMSubArchVersionNumber(Triple)

If reduce this to ArchVersion = getARMSubArchVersionNumber(Triple) and add a test with -target arm-linux-androideabi21 -march=armv7 then everything still passes.

If I'm right you should be able to simplify this and perhaps roll it into the if (Triple.isAndroid() && ArchVersion >= 7) below. If I'm wrong can we add a test case that fails without the ArchVersion = llvm::ARM::parseArchVersion(ArchName) ?

It will also be worth adding tests for a generic target with -march 


Repository:
  rC Clang

https://reviews.llvm.org/D53121