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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
