jojo added inline comments. ================ Comment at: lib/Basic/Targets.cpp:5709 @@ -5715,1 +5708,3 @@ + + return false; } ---------------- echristo wrote: > compnerd wrote: > > Please collapse this: > > > > return Name == "generic" || llvm::AArch64::parseCPUArch(Name) != > > llvm::ARM::AK_INVALID; > ... why doesn't this actually set the cpu? Agree, this looks better.
================ Comment at: lib/Basic/Targets.cpp:5709 @@ -5715,1 +5708,3 @@ + + return false; } ---------------- jojo wrote: > echristo wrote: > > compnerd wrote: > > > Please collapse this: > > > > > > return Name == "generic" || llvm::AArch64::parseCPUArch(Name) != > > > llvm::ARM::AK_INVALID; > > ... why doesn't this actually set the cpu? > Agree, this looks better. Actually,I have no idea also. I just replaced the original implementation with the using of TargetParser. ================ Comment at: lib/Driver/Tools.cpp:2303 @@ +2302,3 @@ + const char *FeatureName = llvm::AArch64::getArchExtFeature(Feature); + if (FeatureName) + Features.push_back(FeatureName); ---------------- compnerd wrote: > I think that you should use the scoped if here: > > if (const char *FeatureName = llvm::AArch64::getArchExtFeature(Feature)) > Features.push_back(FeatureName); Agree,Looks more accurate and concise. ================ Comment at: lib/Driver/Tools.cpp:2324 @@ +2323,3 @@ + unsigned ArchKind = llvm::AArch64::parseCPUArch(CPU); + unsigned Extersion = llvm::AArch64::getDefaultExtensions(CPU,ArchKind); + ---------------- compnerd wrote: > NIT: Space after the comma. Thanks.I will correct it. Repository: rL LLVM http://reviews.llvm.org/D21277 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits