[PATCH] D51650: Implement target_clones multiversioning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Overall, I think this LGTM, but I did find a few nits. Can you fix the clang-format issues? Also, I'd like to see some C++ test coverage that shows how this works on template (partial) specializations, lambdas (with GNU-style syntax), and overloaded functions. If we deviate from the GCC behavior in any cases, it'd be great to capture it in comments (unless you think the deviation is intentional, at which point we should document it in AttrDocs.td, or you think the deviation needs to be fixed before we land it).



================
Comment at: clang/include/clang/Basic/Attr.td:2706
+                      featuresStrs_begin(), featuresStrs_begin() + Index,
+                      [ FeatureStr ](StringRef S) { return S == FeatureStr; });
+    }
----------------



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11289
+          "or a string literal containing a comma-separated list of versions">,
+      InGroup<DiagGroup<"target-clones-mixed-specifiers">>;
+def warn_target_clone_duplicate_options
----------------
Should this ad hoc group be listed within the `FunctionMultiVersioning` group?


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1968-1972
+  if (checkAttrMutualExclusion<TargetAttr>(S, D, AL) ||
+      checkAttrMutualExclusion<TargetClonesAttr>(S, D, AL) ||
+      checkAttrMutualExclusion<CPUDispatchAttr>(S, D, AL) ||
+      checkAttrMutualExclusion<CPUSpecificAttr>(S, D, AL))
+    return;
----------------
This should be handled in Attr.td via a `def : MutualExclusions<[....]>;` if possible.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3274
 static void handleTargetAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (checkAttrMutualExclusion<TargetAttr>(S, D, AL) ||
+      checkAttrMutualExclusion<TargetClonesAttr>(S, D, AL) ||
----------------
Same here.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3342
+static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (checkAttrMutualExclusion<TargetAttr>(S, D, AL) ||
+      checkAttrMutualExclusion<TargetClonesAttr>(S, D, AL) ||
----------------
And here


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D51650/new/

https://reviews.llvm.org/D51650