Add nomerge statement attribute to clang

[zequanwu]

Summary

Add nomerge statement attribute which applies to all call expressions inside the statement.
Related to D78659

[rnk]

clang/include/clang/Basic/Attr.td

1804

Clang supports many function-like things that are not functions, for example, Objective C method declarations are not considered FunctionDecls in the AST. Blocks are another function-like thing. I see below there is a FunctionLike subject type. Lets use that here.

I would like to make it possible to apply this attribute directly to call expressions as well, but I don't see a good example of how to do that in the existing attribute. I'd like to be able to do this, for example:

void __attribute__((noreturn)) bar();
void foo(bool cond) {
  if (cond)
    [[clang::nomerge]] bar();
  else
    [[clang::nomerge]] bar();
}

This gives the user fine-grained control. It allows them to call some existing API, like perhaps RaiseException, of which they do not control the declaration, and mark the calls as unmergeable.

clang/test/CodeGen/attr-nomerge.c

15 ↗

(On Diff #261001)

These numbers can be unstable. Generally the best practice is to do something like this:

// CHECK: declare void @bar() #[[NOMERGEATTR:\d+]]
// CHECK: attributes #[[NOMERGEATTR]] = { nomerge {{.*}} }

This makes the test more readable and more resilient in the case that the attribute set numbers change.

[zequanwu]

clang/include/clang/Basic/Attr.td

1804

It seems like there is no exiting example of call site attributes. https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/CGCall.cpp#L4767. And the function EmitCall may not know existence of call site attributes.

[aaron.ballman]

This should also have Sema tests ensuring that it only applies to the correct subjects, accepts no arguments, etc.

clang/include/clang/Basic/Attr.td

1804

I would like to make it possible to apply this attribute directly to call expressions as well, but I don't see a good example of how to do that in the existing attribute.

You would have to use a statement attribute rather than a declaration attribute for this, and we have a few of those but none that work on call expressions specifically.

One question I have about that idea is whether attributes give you fine enough levels of control (I don't know enough about nomerge to answer this myself). e.g., foo(bar(), bar()); // Do you want to nomerge the bar() calls, the foo() call, or all three? (You can only add the attribute to the statement expression, not to individual expressions.)

[rnk]

clang/include/clang/Basic/Attr.td

1804

I guess what I really wanted was an expression attribute, so you can put it on the calls to bar. :)

But given that that doesn't exist, I think it would be OK to do either one of the following:

• Apply nomerge attribute to all call sites in the whole Stmt
• Apply nomerge only to the first CallExpr found inside the Stmt

I think the simplest implementation is to, in CodeGenFunction, set a flag (NoMerge) on CodeGenFunction when emitting such an attributed statement with this attribute. Then set it back on exit. Something like that.

[rnk]

clang/include/clang/Basic/Attr.td

1804

It's worth considering some test cases where the call expression is not the top-most AST node in the expression:

[[clang::nomerge]] (x = 42, abort());
[[clang::nomerge]] (void)(throwException());

We can probably make an ExprWithTemporaries wrapper, ParenExpr wrapper, or others.

[zequanwu]

change nomerge to statement attribute

[aaron.ballman]

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

clang/include/clang/Basic/AttrDocs.td

356

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.

clang/lib/CodeGen/CGCall.cpp

4825

Missing a full stop at the end of the comment.

clang/lib/CodeGen/CGStmt.cpp

612

Please don't name the declaration Attr as that's a type name.

clang/lib/Sema/SemaStmtAttr.cpp

176

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.

[zequanwu]

Add Sema test.
Check if nomerge attribute has no arguments and statement contains call expressions.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

2766–2767

I think this should be in the IgnoredAttributes group as well.

clang/lib/Sema/SemaStmtAttr.cpp

174

You'll need to check that S is nonnull, children() can return null children in some cases. Also, S should be a const Stmt* and I wouldn't mind a newline before the function definition.

176–180

I think you could replace this with: llvm::any_of(S->children(), hasCallExpr);

185

attribute doesn't meet the usual naming conventions. Can probably just go with NMA or something lame like that.

190

You should be able to just pass A directly here instead.

clang/test/CodeGen/attr-nomerge.cpp

7

I'd appreciate seeing some more complex tests showing the behavior of the attribute. As some examples:

// Does this work on dynamically initialized global statics?
[[clang::nomerge]] static int i = foo() + bar();

void func() {
  // Is the lambda function call operator nomerge, the contained calls within the lambda, or none of the above?
  [[clang::nomerge]] [] { foo(); bar(); }(); 

  // Do we mark the implicit function calls to begin()/end() as nomerge?
  [[clang::nomerge]] for (auto foo : some_container) {}

  // Does it apply to builtins as well?
  [[clang::nomerge]] foo(__builtin_strlen("bar"), __builtin_strlen("bar"));

  // Does it apply to the substatement of a label?
  [[clang::nomerge]] how_about_this: f(bar(), bar());

  // Does it apply across the components of a for statement?
  for (foo(); bar(); baz()) {}
}

[rnk]

clang/lib/CodeGen/CGStmt.cpp

612

Can we use S.hasAttr, or does that not exist for statements?

616

Please use SaveAndRestore, since this could be re-entrant. Consider cases like:

[[clang::nomerge]] foo(1, ({bar(); [[clang::nomerge]] baz(); 2}), qux());

The inner nomerge will "unset" the outer one too early.

This is similar to what will happen if we allow nomerge on compound statements as in:

[[clang::nomerge]] if (cond) {
   foo();
   [[clang::nomerge}] foo();
   foo(); // will not carry nomerge unless we restore to old value
}

clang/lib/CodeGen/CodeGenFunction.h

596

I would put your field after this one, and name it more specific, something like InNoMergeAttributedStmt. This field tracks a similar concept of being within a region

clang/test/CodeGen/attr-nomerge.cpp

7

I think the emergent behavior from the current implementation is reasonable. For lambdas, it applies to the outer call, not the inner lambda body.

I don't see any reason to make this work for global variables currently.

We could try to ban the application of this attribute to compound statements and statments containing them ({}, if, for), but that's a lot of effort. I could go either way.

[zequanwu]

To keep it simple, nomerge attribute should not be applied to decl statement. Since label statement calls ProcessDeclAttributeList to handle attributes, label statement should not have nomerge attribute.

[zequanwu]

clang/lib/CodeGen/CGStmt.cpp

612

Not exist for statements.

[rsmith]

clang/lib/Sema/SemaStmtAttr.cpp

178

This will recurse into too much (eg, the bodies of lambdas and blocks, and unevaluated operands). It would be better to make this a ConstEvaluatedExprVisitor instead (see include/clang/AST/EvaluatedExprVisitor.h).

[zequanwu]

clang/lib/Sema/SemaStmtAttr.cpp

178

Do you mean to create a subclass of ConstEvaluatedExprVisitor to find if the statement has call expression?

[zequanwu]

use ConstStmtVisitor to do recursive lookup for call expression.

[rsmith]

clang/lib/Sema/SemaStmtAttr.cpp

178

This is still recursing into unevaluated operands (eg. the operand of sizeof). Please use ConstEvaluatedExprVisitor instead of ConstStmtVisitor.

[zequanwu]

use ConstEvaluatedExprVisitor to recursively looking up for call expr

[zequanwu]

Ping

[rnk]

I think this looks good and @zequanwu has addressed the concerns of other reviewers. Please wait until Wednesday before pushing in case they raise other concerns.

clang/lib/Sema/SemaStmtAttr.cpp

175

I guess I'm a little concerned that we have to break out CRTP and walk the whole sub statement to make this warning work, but we do that kind of stuff everywhere I guess. :( I wish we had a cheaper way to do this.

[zequanwu]

update test case.

[zequanwu]

landed

clang/include/clang/Basic/Attr.td

 
 def FallThrough : StmtAttr {
   let Spellings = [CXX11<"", "fallthrough", 201603>, C2x<"", "fallthrough">,
                    CXX11<"clang", "fallthrough">, GCC<"fallthrough">];
 //  let Subjects = [NullStmt];
   let Documentation = [FallthroughDocs];
 }
 
+def NoMerge : StmtAttr {
+  let Spellings = [Clang<"nomerge">];
+  let Documentation = [NoMergeDocs];
+}
+
 def FastCall : DeclOrTypeAttr {
   let Spellings = [GCC<"fastcall">, Keyword<"__fastcall">,
                    Keyword<"_fastcall">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [FastCallDocs];
 }
 
 def RegCall : DeclOrTypeAttr {

 def NoReturn : InheritableAttr {
   let Spellings = [GCC<"noreturn">, Declspec<"noreturn">];
   // FIXME: Does GCC allow this on the function instead?
   let Documentation = [Undocumented];
 }
 
 def NoInstrumentFunction : InheritableAttr {
   let Spellings = [GCC<"no_instrument_function">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [Undocumented];
   let SimpleHandler = 1;
 }
 
 def NotTailCalled : InheritableAttr {
   let Spellings = [Clang<"not_tail_called">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [NotTailCalledDocs];

clang/include/clang/Basic/AttrDocs.td

def CXX11NoReturnDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 A function declared as ``[[noreturn]]`` shall not return to its caller. The
 compiler will generate a diagnostic for a function declared as ``[[noreturn]]``
 that appears to be capable of returning to its caller.
   }];
 }
 
+def NoMergeDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+If a statement is marked ``nomerge`` and contains call experessions, those call
+expressions inside the statement will not be merged during optimization. This 
+attribute can be used to prevent the optimizer from obscuring the source
+location of certain calls. For example, it will prevent tail merging otherwise
+identical code sequences that raise an exception or terminate the program. Tail
+merging normally reduces the precision of source location information, making
+stack traces less useful for debugging. This attribute gives the user control
+over the tradeoff between code size and debug information precision.
+  }];
+}
+
 def AssertCapabilityDocs : Documentation {
   let Category = DocCatFunction;
   let Heading = "assert_capability, assert_shared_capability";
   let Content = [{
 Marks a function that dynamically tests whether a capability is held, and halts
 the program if it is not held.
   }];
 }

clang/include/clang/Basic/DiagnosticSemaKinds.td

 // Objective-C++
 def err_objc_decls_may_only_appear_in_global_scope : Error<
   "Objective-C declarations may only appear in global scope">;
 def warn_auto_var_is_id : Warning<
   "'auto' deduced as 'id' in declaration of %0">,
   InGroup<DiagGroup<"auto-var-id">>;
 
 // Attributes
+def warn_nomerge_attribute_ignored_in_stmt: Warning<
+  "%0 attribute is ignored because there exists no call expression inside the "
+  "statement">,
+  InGroup<IgnoredAttributes>;
 def err_nsobject_attribute : Error<
   "'NSObject' attribute is for pointer types only">;
 def err_attributes_are_not_compatible : Error<
   "%0 and %1 attributes are not compatible">;
 def err_attribute_invalid_argument : Error<
   "%select{a reference type|an array type|a non-vector or "
   "non-vectorizable scalar type}0 is an invalid argument to attribute %1">;
 def err_attribute_wrong_number_arguments : Error<

clang/lib/CodeGen/CGCall.cpp

#endif
 
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl))
     if (FD->usesFPIntrin())
       // All calls within a strictfp function are marked strictfp
       Attrs =
         Attrs.addAttribute(getLLVMContext(), llvm::AttributeList::FunctionIndex,
                            llvm::Attribute::StrictFP);
 
+  // Add call-site nomerge attribute if exists.
+  if (InNoMergeAttributedStmt)
+    Attrs =
+      Attrs.addAttribute(getLLVMContext(), llvm::AttributeList::FunctionIndex,
+                         llvm::Attribute::NoMerge);
+
   // Apply some call-site-specific attributes.
   // TODO: work this into building the attribute set.
 
   // Apply always_inline to all calls within flatten functions.
   // FIXME: should this really take priority over __try, below?
   if (CurCodeDecl && CurCodeDecl->hasAttr<FlattenAttr>() &&
       !(TargetDecl && TargetDecl->hasAttr<NoInlineAttr>())) {
     Attrs =

clang/lib/CodeGen/CGStmt.cpp

 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
 using namespace CodeGen;
 
 //===----------------------------------------------------------------------===//
 //                              Statement Emission
 //===----------------------------------------------------------------------===//
 

 
 
 void CodeGenFunction::EmitLabelStmt(const LabelStmt &S) {
   EmitLabel(S.getDecl());
   EmitStmt(S.getSubStmt());
 }
 
 void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
+  bool nomerge = false;
+  for (const auto *A : S.getAttrs())
+    if (A->getKind() == attr::NoMerge) {
+      nomerge = true;
+      break;
+    }
+  SaveAndRestore<bool> save_nomerge(InNoMergeAttributedStmt, nomerge);
   EmitStmt(S.getSubStmt(), S.getAttrs());
 }
 
 void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) {
   // If this code is reachable then emit a stop point (if generating
   // debug info). We have to do this ourselves because we are on the
   // "simple" statement path.
   if (HaveInsertPoint())

clang/lib/CodeGen/CodeGenFunction.h

public:
   const NamedDecl *CurSEHParent = nullptr;
 
   /// True if the current function is an outlined SEH helper. This can be a
   /// finally block or filter expression.
   bool IsOutlinedSEHHelper = false;
 
   /// True if CodeGen currently emits code inside presereved access index
   /// region.
   bool IsInPreservedAIRegion = false;
 
+  /// True if the current statement has nomerge attribute.
+  bool InNoMergeAttributedStmt = false;
+
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
   llvm::DenseMap<const VarDecl *, FieldDecl *> LambdaCaptureFields;
   FieldDecl *LambdaThisCaptureField = nullptr;
 
   /// A mapping from NRVO variables to the flags used to indicate
   /// when the NRVO has been applied to this variable.

clang/lib/Sema/SemaStmtAttr.cpp

 //===--- SemaStmtAttr.cpp - Statement Attribute Handling ------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements stmt-related attribute processing.
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/DelayedDiagnostic.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "llvm/ADT/StringExtras.h"
 
if (Option == LoopHintAttr::VectorizeWidth ||
         llvm_unreachable("bad loop hint argument");
     } else
       llvm_unreachable("bad loop hint");
   }
 
   return LoopHintAttr::CreateImplicit(S.Context, Option, State, ValueExpr, A);
 }
 
+namespace {
+class CallExprFinder : public ConstEvaluatedExprVisitor<CallExprFinder> {
+  bool FoundCallExpr = false;
+
+public:
+  typedef ConstEvaluatedExprVisitor<CallExprFinder> Inherited;
+
+  CallExprFinder(Sema &S, const Stmt *St) : Inherited(S.Context) { Visit(St); }
+
+  bool foundCallExpr() { return FoundCallExpr; }
+
+  void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
+
+  void Visit(const Stmt *St) {
+    if (!St)
+      return;
+    ConstEvaluatedExprVisitor<CallExprFinder>::Visit(St);
+  }
+};
+} // namespace
+
+static Attr *handleNoMergeAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+                               SourceRange Range) {
+  NoMergeAttr NMA(S.Context, A);
+  if (S.CheckAttrNoArgs(A))
+    return nullptr;
+
+  CallExprFinder CEF(S, St);
+
+  if (!CEF.foundCallExpr()) {
+    S.Diag(St->getBeginLoc(), diag::warn_nomerge_attribute_ignored_in_stmt)
+        << NMA.getSpelling();
+    return nullptr;
+  }
+
+  return ::new (S.Context) NoMergeAttr(S.Context, A);
+}
+
 static void
 CheckForIncompatibleAttributes(Sema &S,
                                const SmallVectorImpl<const Attr *> &Attrs) {
   // There are 6 categories of loop hints attributes: vectorize, interleave,
   // unroll, unroll_and_jam, pipeline and distribute. Except for distribute they
   // come in two variants: a state form and a numeric form.  The state form
   // selectively defaults/enables/disables the transformation for the loop
   // (for unroll, default indicates full unrolling rather than enabling the
static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
   case ParsedAttr::AT_FallThrough:
     return handleFallThroughAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
     return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
     return handleOpenCLUnrollHint(S, St, A, Range);
   case ParsedAttr::AT_Suppress:
     return handleSuppressAttr(S, St, A, Range);
+  case ParsedAttr::AT_NoMerge:
+    return handleNoMergeAttr(S, St, A, Range);
   default:
     // if we're here, then we parsed a known attribute, but didn't recognize
     // it as a statement attribute => it is declaration attribute
     S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
         << A << St->getBeginLoc();
     return nullptr;
   }
 }

clang/test/CodeGen/attr-nomerge.cpp

+// RUN: %clang_cc1 -S -emit-llvm %s -triple x86_64-unknown-linux-gnu -o - | FileCheck %s
+
+bool bar();
+void f(bool, bool);
+
+void foo(int i) {
+  [[clang::nomerge]] bar();
+  [[clang::nomerge]] (i = 4, bar());
+  [[clang::nomerge]] (void)(bar());
+  [[clang::nomerge]] f(bar(), bar());
+  [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the anonymous function call
+  [[clang::nomerge]] for (bar(); bar(); bar()) {}
+  bar();
+}
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR:[0-9]+]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call void @_Z1fbb({{.*}}) #[[NOMERGEATTR]]
+// CHECK: call void @"_ZZ3fooiENK3$_0clEv"(%class.anon* %ref.tmp) #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv()
+// CHECK: attributes #[[NOMERGEATTR]] = { nomerge }

clang/test/Sema/attr-nomerge.cpp

+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+void bar();
+
+void foo() {
+  [[clang::nomerge]] bar();
+  [[clang::nomerge(1, 2)]] bar(); // expected-error {{'nomerge' attribute takes no arguments}}
+  int x;
+  [[clang::nomerge]] x = 10; // expected-warning {{nomerge attribute is ignored because there exists no call expression inside the statement}}
+
+  [[clang::nomerge]] label: bar(); // expected-error {{'nomerge' attribute cannot be applied to a declaration}}
+
+}
+
+int f();
+
+[[clang::nomerge]] static int i = f(); // expected-error {{'nomerge' attribute cannot be applied to a declaration}}