[PATCH] Pragma optimize on/off

Dario Domizioli dario.domizioli at gmail.com
Thu May 8 09:43:38 PDT 2014


Hello again!

I have now extended the patch with support for serialization of the state
of the pragma (for PCHs). I have followed a similar pattern to the one used
for the floating point pragmas and the diagnostic pragmas.
I have added a PCH test that verifies that if a "#pragma clang optimize
off" is still active at the end of a PCH then a source file compiled
including the PCH will behave as if the pragma was specified in the source
(as it happens with normal headers). The test is modeled after the pragma
diagnostics test.

I hope the new attached patch covers the issue that was raised by
Richard... but I welcome any feedback. I might have missed something.

Cheers,
    Dario Domizioli
    SN Systems - Sony Computer Entertainment Group








On 7 May 2014 18:41, Dario Domizioli <dario.domizioli at gmail.com> wrote:

>
> Thanks Aaron!
>
> Patch LGTM, modulo Richard's comments about serialization (I worried
>> about the same thing, and my preference is for the patch to be
>> all-inclusive when feasible, but Richard is welcome to weigh in). I
>> have no strong opinions on whether it should be one diagnostic or two.
>>
>
> I will wait for Richard's comments then; meanwhile I'll keep working on
> the serialization issue.
>
> As for the diagnostics, I think that having two might make it easier to
> extend the "unexpected" case if the pragma gains new functionality in the
> future, while the "missing" case is unlikely to change... although the
> %select is quite powerful so I haven't got a strong preference either.
>
> Cheers,
>     Dario Domizioli
>     SN Systems - Sony Computer Entertainment Group
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140508/638e8cef/attachment.html>
-------------- next part --------------
Index: lib/Parse/ParsePragma.cpp
===================================================================
--- lib/Parse/ParsePragma.cpp	(revision 208334)
+++ lib/Parse/ParsePragma.cpp	(working copy)
@@ -131,6 +131,16 @@
                     Token &FirstToken) override;
 };
 
+/// PragmaOptimizeHandler - "\#pragma clang optimize on/off".
+struct PragmaOptimizeHandler : public PragmaHandler {
+  PragmaOptimizeHandler(Sema &S)
+    : PragmaHandler("optimize"), Actions(S) {}
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+                    Token &FirstToken) override;
+private:
+  Sema &Actions;
+};
+
 }  // end namespace
 
 void Parser::initializePragmaHandlers() {
@@ -195,6 +205,9 @@
     MSSection.reset(new PragmaMSPragma("section"));
     PP.AddPragmaHandler(MSSection.get());
   }
+
+  OptimizeHandler.reset(new PragmaOptimizeHandler(Actions));
+  PP.AddPragmaHandler("clang", OptimizeHandler.get());
 }
 
 void Parser::resetPragmaHandlers() {
@@ -249,6 +262,9 @@
 
   PP.RemovePragmaHandler("STDC", FPContractHandler.get());
   FPContractHandler.reset();
+
+  PP.RemovePragmaHandler("clang", OptimizeHandler.get());
+  OptimizeHandler.reset();
 }
 
 /// \brief Handle the annotation token produced for #pragma unused(...)
@@ -1531,3 +1547,40 @@
 
   Actions.ActOnPragmaMSComment(Kind, ArgumentString);
 }
+
+// #pragma clang optimize off
+// #pragma clang optimize on
+void PragmaOptimizeHandler::HandlePragma(Preprocessor &PP, 
+                                        PragmaIntroducerKind Introducer,
+                                        Token &FirstToken) {
+  Token Tok;
+  PP.Lex(Tok);
+  if (Tok.is(tok::eod)) {
+    PP.Diag(Tok.getLocation(), diag::err_pragma_optimize_missing_argument);
+    return;
+  }
+  if (Tok.isNot(tok::identifier)) {
+    PP.Diag(Tok.getLocation(), diag::err_pragma_optimize_invalid_argument)
+      << PP.getSpelling(Tok);
+    return;
+  }
+  const IdentifierInfo *II = Tok.getIdentifierInfo();
+  // The only accepted values are 'on' or 'off'.
+  bool IsOn = false;
+  if (II->isStr("on")) {
+    IsOn = true;
+  } else if (!II->isStr("off")) {
+    PP.Diag(Tok.getLocation(), diag::err_pragma_optimize_invalid_argument)
+      << PP.getSpelling(Tok);
+    return;
+  }
+  PP.Lex(Tok);
+  
+  if (Tok.isNot(tok::eod)) {
+    PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
+      << "clang optimize";
+    return;
+  }
+
+  Actions.ActOnPragmaOptimize(IsOn, FirstToken.getLocation());
+}
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp	(revision 208334)
+++ lib/Serialization/ASTReader.cpp	(working copy)
@@ -3272,6 +3272,14 @@
       LateParsedTemplates.append(Record.begin(), Record.end());
       break;
     }
+
+    case OPTIMIZE_PRAGMA_OPTIONS:
+      if (Record.size() != 1) {
+        Error("invalid pragma optimize record");
+        return Failure;
+      }
+      OptimizeOffPragmaLocation = ReadSourceLocation(F, Record[0]);
+      break;
     }
   }
 }
@@ -6784,6 +6792,11 @@
     }
     SemaDeclRefs.clear();
   }
+
+  // Update the state of 'pragma clang optimize'. Use the same API as if we had
+  // encountered the pragma in the source.
+  bool IsOn = !OptimizeOffPragmaLocation.isValid();
+  SemaObj->ActOnPragmaOptimize(IsOn, OptimizeOffPragmaLocation);
 }
 
 IdentifierInfo* ASTReader::get(const char *NameStart, const char *NameEnd) {
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp	(revision 208334)
+++ lib/Serialization/ASTWriter.cpp	(working copy)
@@ -864,6 +864,7 @@
   RECORD(MACRO_OFFSET);
   RECORD(MACRO_TABLE);
   RECORD(LATE_PARSED_TEMPLATE);
+  RECORD(OPTIMIZE_PRAGMA_OPTIONS);
 
   // SourceManager Block.
   BLOCK(SOURCE_MANAGER_BLOCK);
@@ -3848,6 +3849,14 @@
   Stream.EmitRecord(LATE_PARSED_TEMPLATE, Record);
 }
 
+/// \brief Write the state of 'pragma clang optimize' at the end of the module.
+void ASTWriter::WriteOptimizePragmaOptions(Sema &SemaRef) {
+  RecordData Record;
+  SourceLocation PragmaLoc = SemaRef.getOptimizeOffPragmaLocation();
+  AddSourceLocation(PragmaLoc, Record);
+  Stream.EmitRecord(OPTIMIZE_PRAGMA_OPTIONS, Record);
+}
+
 //===----------------------------------------------------------------------===//
 // General Serialization Routines
 //===----------------------------------------------------------------------===//
@@ -4464,6 +4473,7 @@
   WriteMergedDecls();
   WriteObjCCategories();
   WriteLateParsedTemplates(SemaRef);
+  WriteOptimizePragmaOptions(SemaRef);
 
   // Some simple statistics
   Record.clear();
Index: lib/Sema/SemaAttr.cpp
===================================================================
--- lib/Sema/SemaAttr.cpp	(revision 208334)
+++ lib/Sema/SemaAttr.cpp	(working copy)
@@ -470,6 +470,38 @@
   D->addAttr(CFAuditedTransferAttr::CreateImplicit(Context, Loc));
 }
 
+void Sema::ActOnPragmaOptimize(bool On, SourceLocation PragmaLoc) {
+  if(On)
+    OptimizeOffPragmaLocation = SourceLocation();
+  else
+    OptimizeOffPragmaLocation = PragmaLoc;
+}
+
+SourceLocation Sema::getOptimizeOffPragmaLocation() {
+  return OptimizeOffPragmaLocation;
+}
+
+void Sema::AddRangeBasedOptnone(FunctionDecl *FD) {
+  // In the future, check other pragmas if they're implemented (e.g. pragma
+  // optimize 0 will probably map to this functionality too).
+  if(OptimizeOffPragmaLocation.isValid())
+    AddOptnoneAttributeIfNoConflicts(FD, OptimizeOffPragmaLocation);
+}
+
+void Sema::AddOptnoneAttributeIfNoConflicts(FunctionDecl *FD, 
+                                            SourceLocation Loc) {
+  // Don't add a conflicting attribute. No diagnostic is needed.
+  if (FD->hasAttr<MinSizeAttr>() || FD->hasAttr<AlwaysInlineAttr>())
+    return;
+
+  // Add attributes only if required. Optnone requires noinline as well, but if
+  // either is already present then don't bother adding them.
+  if (!FD->hasAttr<OptimizeNoneAttr>())
+    FD->addAttr(OptimizeNoneAttr::CreateImplicit(Context, Loc));
+  if (!FD->hasAttr<NoInlineAttr>())
+    FD->addAttr(NoInlineAttr::CreateImplicit(Context, Loc));
+}
+
 typedef std::vector<std::pair<unsigned, SourceLocation> > VisStack;
 enum : unsigned { NoVisibility = ~0U };
 
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp	(revision 208334)
+++ lib/Sema/SemaDecl.cpp	(working copy)
@@ -7378,6 +7378,11 @@
   // marking the function.
   AddCFAuditedAttribute(NewFD);
 
+  // If this is a function definition, check if we have to apply optnone due to
+  // a pragma.
+  if(D.isFunctionDefinition())
+    AddRangeBasedOptnone(NewFD);
+
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
   if (NewFD->isFirstDecl() && !NewFD->isInvalidDecl() &&
Index: test/Parser/pragma-optimize-diagnostics.cpp
===================================================================
--- test/Parser/pragma-optimize-diagnostics.cpp	(revision 0)
+++ test/Parser/pragma-optimize-diagnostics.cpp	(revision 0)
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#pragma clang optimize off
+
+#pragma clang optimize on
+
+// Extra arguments
+#pragma clang optimize on top of spaghetti  // expected-warning {{extra tokens at end of '#pragma clang optimize' - ignored}}
+
+// Wrong argument
+#pragma clang optimize something_wrong  // expected-error {{unexpected argument 'something_wrong' to '#pragma clang optimize'; expected 'on' or 'off'}}
+
+// No argument
+#pragma clang optimize // expected-error {{missing argument to '#pragma clang optimize'; expected 'on' or 'off'}}
+
+// Check that macros can be used in the pragma
+#define OFF off
+#define ON on
+#pragma clang optimize OFF
+#pragma clang optimize ON
+
Index: test/SemaCXX/pragma-optimize.cpp
===================================================================
--- test/SemaCXX/pragma-optimize.cpp	(revision 0)
+++ test/SemaCXX/pragma-optimize.cpp	(revision 0)
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-llvm -O2 < %s | FileCheck %s
+
+#pragma clang optimize off
+
+// This is a macro definition and therefore its text is not present after
+// preprocessing. The pragma has no effect here.
+#define CREATE_FUNC(name)        \
+int name (int param) {           \
+    return param;                \
+}                                \
+
+// This is a declaration and therefore it is not decorated with `optnone`.
+extern int foo(int a, int b);
+// CHECK-DAG: @_Z3fooii{{.*}} [[ATTRFOO:#[0-9]+]]
+
+// This is a definition and therefore it will be decorated with `optnone`.
+int bar(int x, int y) {
+    for(int i = 0; i < x; ++i)
+        y += x;
+    return y + foo(x, y);
+}
+// CHECK-DAG: @_Z3barii{{.*}} [[ATTRBAR:#[0-9]+]]
+
+// The function "int created (int param)" created by the macro invocation
+// is also decorated with the `optnone` attribute because it is within a
+// region of code affected by the functionality (not because of the position
+// of the macro definition).
+CREATE_FUNC (created)
+// CHECK-DAG: @_Z7createdi{{.*}} [[ATTRCREATED:#[0-9]+]]
+
+class MyClass {
+    public:
+        // The declaration of the method is not decorated with `optnone`.
+        int method(int blah);
+};
+
+// The definition of the method instead is decorated with `optnone`.
+int MyClass::method(int blah) {
+    return blah + 1;
+}
+// CHECK-DAG: @_ZN7MyClass6methodEi{{.*}} [[ATTRMETHOD:#[0-9]+]]
+
+// A template declaration will not be decorated with `optnone`.
+template <typename T> T twice (T param);
+
+// The template definition will be decorated with the attribute `optnone`.
+template <typename T> T thrice (T param) {
+    return 3 * param;
+}
+
+// This function definition will not be decorated with `optnone` because the
+// attribute would conflict with `always_inline`.
+int __attribute__((always_inline)) baz(int z) {
+    return foo(z, 2);
+}
+// CHECK-DAG: @_Z3bazi{{.*}} [[ATTRBAZ:#[0-9]+]]
+
+#pragma clang optimize on
+
+// The function "int wombat(int param)" created by the macro is not
+// decorated with `optnone`, because the pragma applies its effects only
+// after preprocessing. The position of the macro definition is not
+// relevant.
+CREATE_FUNC (wombat)
+// CHECK-DAG: @_Z6wombati{{.*}} [[ATTRWOMBAT:#[0-9]+]]
+
+// This instantiation of the "twice" template function with a "float" type
+// will not have an `optnone` attribute because the template declaration was
+// not affected by the pragma.
+float container (float par) {
+    return twice(par);
+}
+// CHECK-DAG: @_Z9containerf{{.*}} [[ATTRCONTAINER:#[0-9]+]]
+// CHECK-DAG: @_Z5twiceIfET_S0_{{.*}} [[ATTRTWICE:#[0-9]+]]
+
+// This instantiation of the "thrice" template function with a "float" type
+// will have an `optnone` attribute because the template definition was
+// affected by the pragma.
+float container2 (float par) {
+    return thrice(par);
+}
+// CHECK-DAG: @_Z10container2f{{.*}} [[ATTRCONTAINER2:#[0-9]+]]
+// CHECK-DAG: @_Z6thriceIfET_S0_{{.*}} [[ATTRTHRICEFLOAT:#[0-9]+]]
+
+
+// A template specialization is a new definition and it will not be
+// decorated with an `optnone` attribute because it is now outside of the
+// affected region.
+template<> int thrice(int par) {
+    return (par << 1) + par;
+}
+int container3 (int par) {
+    return thrice(par);
+}
+// CHECK-DAG: @_Z10container3i{{.*}} [[ATTRCONTAINER3:#[0-9]+]]
+// CHECK-DAG: @_Z6thriceIiET_S0_{{.*}} [[ATTRTHRICEINT:#[0-9]+]]
+
+
+// Check for both noinline and optnone on each function that should have them.
+// CHECK-DAG: attributes [[ATTRBAR]] = { {{.*}}noinline{{.*}}optnone{{.*}} }
+// CHECK-DAG: attributes [[ATTRCREATED]] = { {{.*}}noinline{{.*}}optnone{{.*}} }
+// CHECK-DAG: attributes [[ATTRMETHOD]] = { {{.*}}noinline{{.*}}optnone{{.*}} }
+// CHECK-DAG: attributes [[ATTRTHRICEFLOAT]] = { {{.*}}noinline{{.*}}optnone{{.*}} }
+
+// Check that the other functions do NOT have optnone.
+// CHECK-DAG-NOT: attributes [[ATTRFOO]] = { {{.*}}optnone{{.*}} }
+// CHECK-DAG-NOT: attributes [[ATTRBAZ]] = { {{.*}}optnone{{.*}} }
+// CHECK-DAG-NOT: attributes [[ATTRWOMBAT]] = { {{.*}}optnone{{.*}} }
+// CHECK-DAG-NOT: attributes [[ATTRCONTAINER]] = { {{.*}}optnone{{.*}} }
+// CHECK-DAG-NOT: attributes [[ATTRTWICE]] = { {{.*}}optnone{{.*}} }
+// CHECK-DAG-NOT: attributes [[ATTRCONTAINER2]] = { {{.*}}optnone{{.*}} }
+// CHECK-DAG-NOT: attributes [[ATTRCONTAINER3]] = { {{.*}}optnone{{.*}} }
+// CHECK-DAG-NOT: attributes [[ATTRTHRICEINT]] = { {{.*}}optnone{{.*}} }
Index: test/PCH/pragma-optimize.c
===================================================================
--- test/PCH/pragma-optimize.c	(revision 0)
+++ test/PCH/pragma-optimize.c	(revision 0)
@@ -0,0 +1,27 @@
+// Test this without pch.
+// RUN: %clang_cc1 %s -include %s -verify -fsyntax-only
+
+// Test with pch.
+// RUN: %clang_cc1 %s -emit-pch -o %t
+// RUN: %clang_cc1 %s -emit-llvm -include-pch %t -o - | FileCheck %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+#pragma clang optimize off
+
+#else
+
+int a;
+
+void f() {
+  a = 12345;
+}
+
+// Check that the function is decorated with optnone
+
+// CHECK-DAG: @f() [[ATTRF:#[0-9]+]]
+// CHECK-DAG: attributes [[ATTRF]] = { {{.*}}noinline{{.*}}optnone{{.*}} }
+
+#endif
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h	(revision 208334)
+++ include/clang/Parse/Parser.h	(working copy)
@@ -160,6 +160,7 @@
   std::unique_ptr<PragmaHandler> MSConstSeg;
   std::unique_ptr<PragmaHandler> MSCodeSeg;
   std::unique_ptr<PragmaHandler> MSSection;
+  std::unique_ptr<PragmaHandler> OptimizeHandler;
 
   std::unique_ptr<CommentHandler> CommentSemaHandler;
 
Index: include/clang/Serialization/ASTReader.h
===================================================================
--- include/clang/Serialization/ASTReader.h	(revision 208334)
+++ include/clang/Serialization/ASTReader.h	(working copy)
@@ -768,6 +768,9 @@
   /// \brief The floating point pragma option settings.
   SmallVector<uint64_t, 1> FPPragmaOptions;
 
+  /// \brief The pragma clang optimize location (if the pragma state is "off").
+  SourceLocation OptimizeOffPragmaLocation;
+
   /// \brief The OpenCL extension settings.
   SmallVector<uint64_t, 1> OpenCLExtensions;
 
Index: include/clang/Serialization/ASTBitCodes.h
===================================================================
--- include/clang/Serialization/ASTBitCodes.h	(revision 208334)
+++ include/clang/Serialization/ASTBitCodes.h	(working copy)
@@ -542,7 +542,10 @@
       UNDEFINED_BUT_USED = 49,
 
       /// \brief Record code for late parsed template functions.
-      LATE_PARSED_TEMPLATE = 50
+      LATE_PARSED_TEMPLATE = 50,
+
+      /// \brief Record code for \#pragma optimize options.
+      OPTIMIZE_PRAGMA_OPTIONS = 51
     };
 
     /// \brief Record types used within a source manager block.
Index: include/clang/Serialization/ASTWriter.h
===================================================================
--- include/clang/Serialization/ASTWriter.h	(revision 208334)
+++ include/clang/Serialization/ASTWriter.h	(working copy)
@@ -488,6 +488,7 @@
   void WriteRedeclarations();
   void WriteMergedDecls();
   void WriteLateParsedTemplates(Sema &SemaRef);
+  void WriteOptimizePragmaOptions(Sema &SemaRef);
 
   unsigned DeclParmVarAbbrev;
   unsigned DeclContextLexicalAbbrev;
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td	(revision 208334)
+++ include/clang/Basic/DiagnosticParseKinds.td	(working copy)
@@ -843,6 +843,12 @@
 def err_pragma_pointers_to_members_unknown_kind : Error<
   "unexpected %0, expected to see one of %select{|'best_case', 'full_generality', }1"
   "'single_inheritance', 'multiple_inheritance', or 'virtual_inheritance'">;
+// - #pragma clang optimize on/off
+def err_pragma_optimize_missing_argument : Error<
+  "missing argument to '#pragma clang optimize'; expected 'on' or 'off'">;
+def err_pragma_optimize_invalid_argument : Error<
+  "unexpected argument '%0' to '#pragma clang optimize'; "
+  "expected 'on' or 'off'">;
 
 // OpenCL Section 6.8.g
 def err_not_opencl_storage_class_specifier : Error<
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h	(revision 208334)
+++ include/clang/Sema/Sema.h	(working copy)
@@ -333,6 +333,11 @@
   /// VisContext - Manages the stack for \#pragma GCC visibility.
   void *VisContext; // Really a "PragmaVisStack*"
 
+  /// \brief This represents the last location of a "#pragma clang optimize off"
+  /// directive if such a directive has not been closed by an "on" yet. If
+  /// optimizations are currently "on", this is set to an invalid location.
+  SourceLocation OptimizeOffPragmaLocation;
+
   /// \brief Flag indicating if Sema is building a recovery call expression.
   ///
   /// This flag is used to avoid building recovery call expressions
@@ -7228,6 +7233,23 @@
   /// the appropriate attribute.
   void AddCFAuditedAttribute(Decl *D);
 
+  /// \brief Called on well formed \#pragma clang optimize.
+  void ActOnPragmaOptimize(bool On, SourceLocation PragmaLoc);
+
+  /// \brief Get the location for the currently active "\#pragma clang optimize
+  /// off". If this location is invalid, then the state of the pragma is "on".
+  SourceLocation getOptimizeOffPragmaLocation();
+
+  /// \brief 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.
+  void AddRangeBasedOptnone(FunctionDecl *FD);
+
+  /// \brief Adds the 'optnone' attribute to the function declaration if there
+  /// are no conflicts; Loc represents the location causing the 'optnone'
+  /// attribute to be added (usually because of a pragma).
+  void AddOptnoneAttributeIfNoConflicts(FunctionDecl *FD, SourceLocation Loc);
+
   /// AddAlignedAttr - Adds an aligned attribute to a particular declaration.
   void AddAlignedAttr(SourceRange AttrRange, Decl *D, Expr *E,
                       unsigned SpellingListIndex, bool IsPackExpansion);


More information about the cfe-commits mailing list