[PATCH] Pragma optimize on/off

Richard Smith richard at metafoo.co.uk
Tue May 6 14:26:22 PDT 2014


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.

Generally, this patch looks really good. Thanks for the thorough set of
test cases!


+void PragmaOptimizeHandler::HandlePragma(Preprocessor &PP,
+                                        PragmaIntroducerKind Introducer,
+                                        Token &FirstToken) {
+  bool isOn = false;

Style: Please start variable names with an uppercase letter. Also, I'd
prefer for this variable to be moved down next to where it's initialized.


+  PP.Lex(Tok);

Should we perform macro expansion on this token or not?


+  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/20140506/5eb6dcea/attachment.html>


More information about the cfe-commits mailing list