[PATCH] D21277: Resubmit r270688: Using new TargetParser in Clang.
jojo.ma via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 14 01:46:07 PDT 2016
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
More information about the cfe-commits
mailing list