[PATCH] D79121: Add nomerge function attribute to clang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is missing Sema tests ensuring that the attribute only appertains to the correct subject, does not accept arguments, etc.



================
Comment at: clang/include/clang/Basic/AttrDocs.td:356
+  let Content = [{
+Calls to functions marked `nomerge` will not be merged during optimization.
+This attribute can be used to prevent the optimizer from obscuring the source
----------------
Looks like the docs need updating still. The functions aren't marked `nomerge`, statements are. This should explain the behavior of how we are lowering the `nomerge` attribute on call sites.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:4526
 
+  // Add call-site nomerge attribute if exists
+  if (NoMerge)
----------------
Missing a full stop at the end of the comment.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:611
 void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
+  for (const auto *Attr: S.getAttrs())
+    if (Attr->getKind() == attr::NoMerge) {
----------------
Please don't name the declaration `Attr` as that's a type name.


================
Comment at: clang/lib/Sema/SemaStmtAttr.cpp:175
+                               SourceRange Range) {
+  return ::new (S.Context) NoMergeAttr(S.Context, A);
+}
----------------
Should there be semantic checking here? For instance, I would expect that writing a `nomerge` attribute on a statement which contains no call expressions should be diagnosed because something unexpected has happened.


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

https://reviews.llvm.org/D79121