[PATCH] Pragma optimize on/off
Dario Domizioli
dario.domizioli at gmail.com
Wed May 7 06:24:29 PDT 2014
Hi Richard.
Thanks for the quick reply and for the review!
I am attaching a new patch which fixes the style issues and adds a specific
test case for all the expected diagnostics coming from the handling of
'#pragma clang optimize'.
On 6 May 2014 22:26, Richard Smith <richard at metafoo.co.uk> wrote:
> What should happen if a preprocessed header or a TU preamble ends with a
> "#pragma clang optimize off" directive active? For a module, I don't think
> it should have any effect, but in those other two cases I think we should
> reactivate the directive when loading the preamble or PCH.
>
Yes, if we reach the end of a compilation unit and the state of the pragma
is still "off" then nothing special happens. This is because users may want
to disable optimizations in a whole file by just having '#pragma clang
optimize off' at the top of the file, and they shouldn't need a matching
"on" at the end.
However, you make a good point about PCH. I haven't looked at this in
detail, but I suppose we would have to somehow serialize the state of
OptimizeOffPragmaLocation in the precompiled header. This should be enough,
because any function definition in the header (if any is present) would
have been decorated with 'optnone' already, so at the point of
serialization the effect of the pragma has already been applied and we just
serialize the attributes as usual.
At the moment the patch does not include serialization in the PCH, but I
will have a look at how it's done and implement that too.
Would you prefer to have just one patch with everything in it, or would you
be OK with getting this in and then incrementally improve it with another
patch?
> + PP.Lex(Tok);
>
> Should we perform macro expansion on this token or not?
>
Apparently, by the time the token gets there it has already been expanded.
I have verified this in the additional test case, with:
#define OFF off
#define ON on
#pragma clang optimize OFF
#pragma clang optimize ON
It does not seem to produce any diagnostic, so the tokens are processed as
'off' and 'on'.
If you are OK with the updated patch, could you please commit it for me? (I
don't have commit access yet)
Cheers,
Dario Domizioli
SN Systems - Sony Computer Entertainment Group
>
>
> + if (II->isStr("on")) {
> + isOn = true;
> + PP.Lex(Tok);
> + }
> + else if (II->isStr("off"))
> + PP.Lex(Tok);
> + else {
> + PP.Diag(Tok.getLocation(), diag::err_pragma_optimize_malformed);
> + return;
> + }
>
> Please sink the PP.Lex call below this if/else block. Also, you don't seem
> to have any test coverage for the 'malformed' case.
>
>
> + /// OptimizeOffPragmaLocation - it represents the last location of a
> "#pragma
> + /// clang optimize off" directive if such directive has not been closed
> by an
> + /// "on" yet. If optimizations are currently "on", this is set to an
> invalid
> + /// location.
> + SourceLocation OptimizeOffPragmaLocation;
>
> Style: Please start this comment with "\brief" instead of the name of the
> entity being declared. (Likewise for the next three declarations.)
>
>
> On Tue, May 6, 2014 at 6:38 AM, Dario Domizioli <dario.domizioli at gmail.com
> > wrote:
>
>> Hi,
>>
>> Please review the attached patch for adding "pragma clang optimize
>> on/off" to clang.
>> This pragma is implemented by decorating function definitions in the
>> "off" regions with attribute 'optnone'.
>>
>> The syntax and semantics have been discussed in here:
>> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-April/036561.html
>> The final consensus was that the feature was worth having, and after a
>> while I think we converged on the syntax, so I am now sending the patch for
>> review.
>>
>> Here is a very short summary of how the pragma works:
>>
>> // The default status is "on".
>> #pragma clang optimize off
>> // Function definitions in here are decorated with attribute
>> 'optnone'
>> // but only if it does not conflict with existing attributes.
>> #pragma clang optimize on
>> // Code compiled as normal.
>>
>> The test provided with the patch checks a number of interesting cases,
>> including template definitions and instantiations, macros, and conflicting
>> attributes (e.g. always_inline).
>>
>> I will prepare another patch for the documentation.
>>
>> Cheers,
>> Dario Domizioli
>> SN Systems - Sony Computer Entertainment Group
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140507/3f9afdb8/attachment.html>
-------------- next part --------------
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp (revision 208197)
+++ 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: lib/Sema/SemaAttr.cpp
===================================================================
--- lib/Sema/SemaAttr.cpp (revision 208197)
+++ lib/Sema/SemaAttr.cpp (working copy)
@@ -470,6 +470,34 @@
D->addAttr(CFAuditedTransferAttr::CreateImplicit(Context, Loc));
}
+void Sema::ActOnPragmaOptimize(bool On, SourceLocation PragmaLoc) {
+ if(On)
+ OptimizeOffPragmaLocation = SourceLocation();
+ else
+ OptimizeOffPragmaLocation = PragmaLoc;
+}
+
+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/Parse/ParsePragma.cpp
===================================================================
--- lib/Parse/ParsePragma.cpp (revision 208197)
+++ 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,35 @@
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.isNot(tok::identifier)) {
+ PP.Diag(Tok.getLocation(), diag::err_pragma_optimize_malformed);
+ 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_malformed);
+ 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: 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/SemaCXX/pragma-optimize-diagnostics.cpp
===================================================================
--- test/SemaCXX/pragma-optimize-diagnostics.cpp (revision 0)
+++ test/SemaCXX/pragma-optimize-diagnostics.cpp (revision 0)
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#pragma clang optimize off
+
+#pragma clang optimize on
+
+#pragma clang optimize on top of spaghetti // expected-warning {{extra tokens at end of '#pragma clang optimize' - ignored}}
+
+#pragma clang optimize something_wrong // expected-error {{malformed argument to '#pragma clang optimize' - expected 'on' or 'off'}}
+
+#pragma clang optimize // expected-error {{malformed argument to '#pragma clang optimize' - expected 'on' or 'off'}}
+
+#define OFF off
+#define ON on
+
+#pragma clang optimize OFF
+
+#pragma clang optimize ON
+
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h (revision 208197)
+++ 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/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td (revision 208197)
+++ include/clang/Basic/DiagnosticParseKinds.td (working copy)
@@ -841,6 +841,9 @@
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_malformed : Error<
+ "malformed argument 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 208197)
+++ 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 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,19 @@
/// the appropriate attribute.
void AddCFAuditedAttribute(Decl *D);
+ /// \brief Called on well formed \#pragma clang optimize.
+ void ActOnPragmaOptimize(bool On, SourceLocation PragmaLoc);
+
+ /// \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