[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