[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: lib/CodeGen/CodeGenFunction.cpp:2271
+static bool hasRequiredFeature(StringRef Feature,
+                               llvm::StringMap<bool>& CallerFeatureMap,
+                               std::string &Missing) {
----------------
'&' should be in front of the variable name not after the type.


================
Comment at: lib/CodeGen/CodeGenFunction.cpp:2305
+		      return Feature == ExplicitFeature.substr(1);
+                    }))
+        if (!hasRequiredFeature(Feature, CallerFeatureMap, FirstMissing))
----------------
Please run clang-format on this. There's some weird indentation happening.


================
Comment at: lib/CodeGen/CodeGenFunction.cpp:2345
     StringRef(FeatureList).split(ReqFeatures, ',');
-    if (!hasRequiredFeatures(ReqFeatures, CGM, FD, MissingFeature))
+    if (!hasRequiredFeatures(ReqFeatures, CGM, FD, TargetDecl, MissingFeature))
       CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature)
----------------
Can we call getAttr here and pass the result of that instead of the TargetDecl? We know the attribute exists. Then rename getFunctionTargetAttrs and make it only responsible for removing unknown features. Push its other call site back into the 'if' there. This removes the need to return an empty ParsedTargetAttr.


================
Comment at: lib/CodeGen/CodeGenModule.cpp:4998
 
-// Fills in the supplied string map with the set of target features for the
-// passed in function.
-void CodeGenModule::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
-                                          const FunctionDecl *FD) {
-  StringRef TargetCPU = Target.getTargetOpts().CPU;
+TargetAttr::ParsedTargetAttr CodeGenModule::getFunctionTargetAttrs(const FunctionDecl *FD)
+{
----------------
Open curly brace should be on previous line.


================
Comment at: lib/CodeGen/CodeGenModule.cpp:5024
+  TargetAttr::ParsedTargetAttr ParsedAttr = getFunctionTargetAttrs(FD);
+  if (ParsedAttr.Features.size() != 0) {
     // Make a copy of the features as passed on the command line into the
----------------
!ParsedAttr.Features.empty()


Repository:
  rC Clang

https://reviews.llvm.org/D46541