StephenTozer created this revision.
StephenTozer added reviewers: dblaikie, aprantl, echristo.
StephenTozer added a project: debug-info.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
StephenTozer requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

This patch is part of a set of patches that add an `-fextend-lifetimes` flag to 
clang, which extends the lifetimes of local variables and parameters for 
improved debuggability. In addition to that flag, the patch series adds a new 
function attribute to disable the post-RA scheduler (which is applied by 
`-fextend-lifetimes`), a pragma to selectively disable `-fextend-lifetimes`, 
and an `-fextend-this-ptr` flag which functions as `-fextend-lifetimes` for 
`this` pointers only. All changes and tests in these patches were written by 
@wolfgangp, though I will be managing these reviews and addressing any comments 
raised. Discussion on the approach of this patch series as a whole should be 
directed to the 3rd patch, D157613 <https://reviews.llvm.org/D157613>, in order 
to simplify review.

---

This particular patch adds a new pragma, `pragma clang extend_lifetimes 
disable/enable` that can be used to toggle `-fextend-lifetimes` for subsequent 
functions:

It may be useful for developers to keep certain functions (e.g. very hot 
function that do not need debugging) from having their variable lifetimes 
extended (which will hurt performance). The way to do this is to add `#pragma 
clang extend_lifetimes disable` prior to those functions, and `#pragma clang 
extend_lifetimes enable` afterwards. This is helpful during developer builds 
where the intention is to repeatedly edit-compile-debug, but the developer 
likely does not want `#pragma clang extend_lifetimes enable` to be extending 
variable lifetimes during Release builds. Therefore these pragmas will only 
have an effect when `-fextend-lifetimes` is passed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157618

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGen/pragma-extend-lifetimes.cpp

Index: clang/test/CodeGen/pragma-extend-lifetimes.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGen/pragma-extend-lifetimes.cpp
@@ -0,0 +1,87 @@
+// RUN: %clang %s --target=%itanium_abi_triple -S -O2 -emit-llvm -o - \
+// RUN:   | FileCheck %s --check-prefix=NO-EXTENSION --implicit-check-not="call void (...) @llvm.fake.use"
+// Check that we do not generate fake_use instructions without -fextend-this-ptr
+// or -fextend-lifetimes, even with `#pragma clang extend_lifetimes enable`
+// declared.
+
+// RUN: %clang %s --target=%itanium_abi_triple -S -O2 -emit-llvm -fextend-this-ptr -o - \
+// RUN:   | FileCheck %s --implicit-check-not="call void (...) @llvm.fake.use"
+// Check that we generate a fake_use instruction for this 'this' ptr in foo()
+// and baz(), but not for bar().
+
+// RUN: %clang %s --target=%itanium_abi_triple -S -O2 -emit-llvm -fextend-lifetimes -o - \
+// RUN:   | FileCheck %s --check-prefixes CHECK,ALL --implicit-check-not="call void (...) @llvm.fake.use"
+// Check that we generate a fake_use instruction for all parameters (including
+// the 'this' ptr) in foo() and baz(), but not for bar().
+
+class A {
+  void foo(int i);
+  void bar(int i);
+  void bat(int i);
+  void baz(int i);
+  void goo(int i);
+  void moo(int i);
+  void zoo(int i);
+  void fie(int i);
+};
+
+// NO-EXTENSION: define{{.*}}foo
+void A::foo(int i) {
+  // CHECK-LABEL: define{{.*}}foo
+  // ALL:         call void (...) @llvm.fake.use(i32 %
+  // CHECK:       call void (...) @llvm.fake.use(ptr nonnull %
+}
+
+#pragma clang extend_lifetimes disable
+void A::bar(int i) {
+  // CHECK-LABEL: define{{.*}}bar
+  // CHECK:       ret
+}
+
+void A::bat(int i) {
+// Make sure that a disabling pragma spans multiple function definitions.
+  // CHECK-LABEL: define{{.*}}bat
+  // CHECK:       ret
+}
+
+#pragma clang extend_lifetimes enable
+void A::baz(int i) {
+  // CHECK-LABEL: define{{.*}}baz
+  // ALL:         call void (...) @llvm.fake.use(i32 %
+  // CHECK:       call void (...) @llvm.fake.use(ptr nonnull %
+}
+
+void A::goo(int i) {
+// Make sure that a disabling pragma has no effect on the current function
+// when it appears in the middle.
+#pragma clang extend_lifetimes disable
+  // CHECK-LABEL: define{{.*}}goo
+  // ALL:         call void (...) @llvm.fake.use(i32 %
+  // CHECK:       call void (...) @llvm.fake.use(ptr nonnull %
+}
+
+void A::moo(int i) {
+// Make sure
+// 1) that a disabling pragma that appeared in the middle of the previous
+// function affects the current function.
+// 2) that an enabling pragma that appears in the middle of the current
+// function does not affect the current function.
+#pragma clang extend_lifetimes enable
+  // CHECK-LABEL: define{{.*}}moo
+  // CHECK:       ret
+}
+
+void A::zoo(int i) {
+// Finally, make sure that an enabling pragma that appeared in the middle of
+// the previous function affects the current function.
+  // CHECK-LABEL: define{{.*}}zoo
+  // ALL:         call void (...) @llvm.fake.use(i32 %
+  // CHECK:       call void (...) @llvm.fake.use(ptr nonnull %
+}
+
+void __attribute__((nodebug)) A::fie(int i) {
+  int j = 0;
+// Make sure that the nodebug attribute prevents extend-lifetimes from
+// taking effect.
+  // CHECK-LABEL: define{{.*}}fie
+}
Index: clang/lib/Sema/SemaLambda.cpp
===================================================================
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -1317,6 +1317,9 @@
   // have to apply optnone due to a pragma.
   AddRangeBasedOptnone(Method);
 
+  // Check if we have to apply the extend_lifetimes disable pragma.
+  AddRangeBasedExtendLifetimesDisable(Method);
+
   // code_seg attribute on lambda apply to the method.
   if (Attr *A = getImplicitCodeSegOrSectionAttrForFunction(
           Method, /*IsDefinition=*/true))
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10639,6 +10639,10 @@
     ModifyFnAttributesMSPragmaOptimize(NewFD);
   }
 
+  // Check if we have to apply the extendlifetimes disable pragma.
+  if (D.isFunctionDefinition())
+    AddRangeBasedExtendLifetimesDisable(NewFD);
+
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
   if (NewFD->isFirstDecl() && !NewFD->isInvalidDecl() &&
Index: clang/lib/Sema/SemaAttr.cpp
===================================================================
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1143,6 +1143,13 @@
     OptimizeOffPragmaLocation = PragmaLoc;
 }
 
+void Sema::ActOnPragmaExtendLifetimes(bool Enable, SourceLocation PragmaLoc) {
+  if (Enable)
+    ExtendLifetimesDisablePragmaLocation = SourceLocation();
+  else
+    ExtendLifetimesDisablePragmaLocation = PragmaLoc;
+}
+
 void Sema::ActOnPragmaMSOptimize(SourceLocation Loc, bool IsOn) {
   if (!CurContext->getRedeclContext()->isFileContext()) {
     Diag(Loc, diag::err_pragma_expected_file_scope) << "optimize";
@@ -1206,6 +1213,19 @@
     FD->addAttr(NoInlineAttr::CreateImplicit(Context, Loc));
 }
 
+void Sema::AddRangeBasedExtendLifetimesDisable(FunctionDecl *FD) {
+  if (ExtendLifetimesDisablePragmaLocation.isValid())
+    AddExtendLifetimesDisableAttribute(FD,
+                                       ExtendLifetimesDisablePragmaLocation);
+}
+
+void Sema::AddExtendLifetimesDisableAttribute(FunctionDecl *FD,
+                                              SourceLocation Loc) {
+  // Add the attribute only if it is not already present.
+  if (!FD->hasAttr<ExtendLifetimesDisableAttr>())
+    FD->addAttr(ExtendLifetimesDisableAttr::CreateImplicit(Context, Loc));
+}
+
 void Sema::AddImplicitMSFunctionNoBuiltinAttr(FunctionDecl *FD) {
   SmallVector<StringRef> V(MSFunctionNoBuiltins.begin(),
                            MSFunctionNoBuiltins.end());
Index: clang/lib/Parse/ParsePragma.cpp
===================================================================
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -235,6 +235,18 @@
   Sema &Actions;
 };
 
+/// PragmaExtendLifetimesHandler - "\#pragma clang extend_lifetimes
+/// enable/disable".
+struct PragmaExtendLifetimesHandler : public PragmaHandler {
+  PragmaExtendLifetimesHandler(Sema &S)
+      : PragmaHandler("extend_lifetimes"), Actions(S) {}
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
+                    Token &FirstToken) override;
+
+private:
+  Sema &Actions;
+};
+
 struct PragmaLoopHintHandler : public PragmaHandler {
   PragmaLoopHintHandler() : PragmaHandler("loop") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
@@ -477,6 +489,10 @@
   OptimizeHandler = std::make_unique<PragmaOptimizeHandler>(Actions);
   PP.AddPragmaHandler("clang", OptimizeHandler.get());
 
+  ExtendLifetimesHandler =
+      std::make_unique<PragmaExtendLifetimesHandler>(Actions);
+  PP.AddPragmaHandler("clang", ExtendLifetimesHandler.get());
+
   LoopHintHandler = std::make_unique<PragmaLoopHintHandler>();
   PP.AddPragmaHandler("clang", LoopHintHandler.get());
 
@@ -611,6 +627,9 @@
   PP.RemovePragmaHandler("clang", OptimizeHandler.get());
   OptimizeHandler.reset();
 
+  PP.RemovePragmaHandler("clang", ExtendLifetimesHandler.get());
+  ExtendLifetimesHandler.reset();
+
   PP.RemovePragmaHandler("clang", LoopHintHandler.get());
   LoopHintHandler.reset();
 
@@ -3188,6 +3207,47 @@
   Actions.ActOnPragmaOptimize(IsOn, FirstToken.getLocation());
 }
 
+// #pragma clang extend_lifetimes disable
+// #pragma clang extend_lifetimes enable
+void PragmaExtendLifetimesHandler::HandlePragma(Preprocessor &PP,
+                                                PragmaIntroducer Introducer,
+                                                Token &FirstToken) {
+  Token Tok;
+  PP.Lex(Tok);
+  if (Tok.is(tok::eod)) {
+    PP.Diag(Tok.getLocation(), diag::err_pragma_missing_argument)
+        << "clang extend_lifetimes" << /*Expected=*/true
+        << "'enable' or 'disable'";
+    return;
+  }
+  if (Tok.isNot(tok::identifier)) {
+    PP.Diag(Tok.getLocation(),
+            diag::err_pragma_extend_lifetimes_invalid_argument)
+        << PP.getSpelling(Tok);
+    return;
+  }
+  const IdentifierInfo *II = Tok.getIdentifierInfo();
+  // The only accepted values are 'enable' or 'disable'.
+  bool IsEnabled = false;
+  if (II->isStr("enable")) {
+    IsEnabled = true;
+  } else if (!II->isStr("disable")) {
+    PP.Diag(Tok.getLocation(),
+            diag::err_pragma_extend_lifetimes_invalid_argument)
+        << PP.getSpelling(Tok);
+    return;
+  }
+  PP.Lex(Tok);
+
+  if (Tok.isNot(tok::eod)) {
+    PP.Diag(Tok.getLocation(), diag::err_pragma_extend_lifetimes_extra_argument)
+        << PP.getSpelling(Tok);
+    return;
+  }
+
+  Actions.ActOnPragmaExtendLifetimes(IsEnabled, FirstToken.getLocation());
+}
+
 namespace {
 /// Used as the annotation value for tok::annot_pragma_fp.
 struct TokFPAnnotValue {
Index: clang/lib/CodeGen/CGDecl.cpp
===================================================================
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1473,6 +1473,9 @@
   // Do not extend variables in nodebug functions.
   if (FuncDecl->hasAttr<NoDebugAttr>())
     return false;
+  // pragma clang extend_lifetimes disable has been seen.
+  if (FuncDecl->hasAttr<ExtendLifetimesDisableAttr>())
+    return false;
   return true;
 }
 
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -780,6 +780,12 @@
   /// optimizations are currently "on", this is set to an invalid location.
   SourceLocation OptimizeOffPragmaLocation;
 
+  /// This represents the last location of a "#pragma clang extend_lifetimes
+  /// disable" directive if such a directive has not been closed by an "enable"
+  /// yet. If extend_lifetimes is currently enabled, this is set to an invalid
+  /// location.
+  SourceLocation ExtendLifetimesDisablePragmaLocation;
+
   /// The "on" or "off" argument passed by \#pragma optimize, that denotes
   /// whether the optimizations in the list passed to the pragma should be
   /// turned off or on. This boolean is true by default because command line
@@ -10918,6 +10924,9 @@
   /// Called on well formed \#pragma clang optimize.
   void ActOnPragmaOptimize(bool On, SourceLocation PragmaLoc);
 
+  /// Called on well formed \#pragma clang extend_lifetimes.
+  void ActOnPragmaExtendLifetimes(bool Enable, SourceLocation PragmaLoc);
+
   /// #pragma optimize("[optimization-list]", on | off).
   void ActOnPragmaMSOptimize(SourceLocation Loc, bool IsOn);
 
@@ -10932,6 +10941,13 @@
     return OptimizeOffPragmaLocation;
   }
 
+  /// Get the location for the currently active "\#pragma extend_lifetimes
+  /// disable". If this location is invalid, then the state of the pragma is
+  /// "enabled".
+  SourceLocation getExtendLifetimesDisablePragmaLocation() const {
+    return ExtendLifetimesDisablePragmaLocation;
+  }
+
   /// Only called on function definitions; if there is a pragma in scope
   /// with the effect of a range-based optnone, consider marking the function
   /// with attribute optnone.
@@ -10947,6 +10963,14 @@
   /// attribute to be added (usually because of a pragma).
   void AddOptnoneAttributeIfNoConflicts(FunctionDecl *FD, SourceLocation Loc);
 
+  /// For function definitions, if there is a pragma in scope with the effect
+  /// of disabling extend_lifetimes, mark the function with attribute
+  /// ExtendLifetimesDisable.
+  void AddRangeBasedExtendLifetimesDisable(FunctionDecl *FD);
+
+  /// Adds the ExtendLifetimesDisable attribute to the function declaration.
+  void AddExtendLifetimesDisableAttribute(FunctionDecl *FD, SourceLocation Loc);
+
   /// Only called on function definitions; if there is a MSVC #pragma optimize
   /// in scope, consider changing the function's attributes based on the
   /// optimization list passed to the pragma.
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -206,6 +206,7 @@
   std::unique_ptr<PragmaHandler> MSAllocText;
   std::unique_ptr<PragmaHandler> CUDAForceHostDeviceHandler;
   std::unique_ptr<PragmaHandler> OptimizeHandler;
+  std::unique_ptr<PragmaHandler> ExtendLifetimesHandler;
   std::unique_ptr<PragmaHandler> LoopHintHandler;
   std::unique_ptr<PragmaHandler> UnrollHintHandler;
   std::unique_ptr<PragmaHandler> NoUnrollHintHandler;
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1268,6 +1268,12 @@
   "expected 'on' or 'off'">;
 def err_pragma_optimize_extra_argument : Error<
   "unexpected extra argument '%0' to '#pragma clang optimize'">;
+// - #pragma clang extend_lifetimes enable/disable
+def err_pragma_extend_lifetimes_invalid_argument : Error<
+  "unexpected argument '%0' to '#pragma clang extend_lifetimes'; "
+  "expected 'enable' or 'disable'">;
+def err_pragma_extend_lifetimes_extra_argument : Error<
+  "unexpected extra argument '%0' to '#pragma clang extend_lifetimes'">;
 // - #pragma clang attribute
 def err_pragma_attribute_expected_push_pop_paren : Error<
   "expected 'push', 'pop', or '(' after '#pragma clang attribute'">;
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3605,6 +3605,32 @@
   }];
 }
 
+def ExtendLifetimesDisableDocs : Documentation {
+  let Category = DocCatFunction;
+  let Heading = "#pragma clang extend_lifetimes disable, #pragma clang extend_lifetimes enable";
+  let Content = [{
+The ``#pragma clang extend_lifetimes disable`` directive can be used to
+selectively disable the ``-fextend-lifetimes`` and ``-fextend-this-ptr`` flags.
+This may be useful in cases where one of these options is used for development
+but results in very poor codegen for some functions. Functions defined after
+this pragma will have extend lifetimes/`this` suppressed;
+``#pragma clang extend_lifetimes enable`` can be used to reenable the lifetime
+extension for subsequent functions definitions. The directive can be used as
+follows:
+
+.. code-block:: c++
+
+  #pragma clang extend_lifetimes disable
+  void foo(...) {} // -fextend-lifetimes disabled for foo,
+  void bar(...) {} // also disabled for bar.
+  #pragma clang extend_lifetimes enable
+  void baz(...) {} // -fextend-lifetimes reenabled for baz.
+
+This directive has no effect if neither ``-fextend-lifetimes` nor
+`-fextend-this-ptr` are passed.
+  }];
+}
+
 def OptnoneDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to