[MSVC] Add initial support for MSVC pragma optimize

[steplong]

Summary

MSVC's pragma optimize turns optimizations on or off based on the list
passed. At the moment, we only support an empty optimization list.

i.e. #pragma optimize("", on | off)

From MSVC's docs:

Parameter	Type of optimization
g	Enable global optimizations. Deprecated
s or t	Specify short or fast sequences of machine code
y	Generate frame pointers on the program stack

https://docs.microsoft.com/en-us/cpp/preprocessor/optimize?view=msvc-170

[aaron.ballman]

clang/lib/Parse/ParsePragma.cpp

3661–3662

I think we should be syntax checking the string. Microsoft doesn't, but... what's the benefit to silently doing nothing?

3662

clang/lib/Sema/SemaAttr.cpp

1099

I got tripped up below by seeing [On] and thinking it only ever looped over the "enabled" items in the list.

1108

It looks like we're not handling this bit of the MS documentation:

Using the optimize pragma with the empty string ("") is a special form of the directive:

...

When you use the on parameter, it resets the optimizations to the ones that you specified using the /O compiler option.

1195

Can you explain why this is needed?

[aaron.ballman]

clang/lib/Sema/SemaAttr.cpp

1178–1184

Rather than creating two new, implicit attributes for this, why not add support for __attribute__((optimize(...))) from GCC and reuse that one?

https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes

It seems like that serves the same function as these implicit attributes, but then users would get two features for the price of one.

[steplong]

clang/lib/Sema/SemaAttr.cpp

1178–1184

Hmm that makes sense. So, pragma optimize(on, "s") would be equivalent to __attribute__((optimize("-Os")). Could you give me a brief description of what I would have to do to implement the attribute? I'm new to this

1195

I added this attr, so we can remove the attr optnone even if -O0 was passed on the cmdline

[rnk]

I think it's fine to implement this, but I wanted to share some of the motivation for the current state of things. In our experience, the majority of uses of pragma optimize were to work around MSVC compiler bugs. So, instead of honoring them, we felt it was best to ignore them with a warning (-Wignored-pragma-optimize). Of course, that warning is typically disabled in build systems of large projects, so our current behavior can still surprise users. Implementing the feature is probably the most predictable and least surprising thing Clang can do.

Something we can't easily support for either GCC or MSVC attribute is enabling optimizations for some functions in an -O0 build. Maybe it's now possible to set up a full pass pipeline after semantic analysis is complete, but it's not straightforward. You should consider what to do with that corner case.

clang/lib/Parse/ParsePragma.cpp

3706

This diagnostic is probably dead now, please remove it from the .td file.

[aaron.ballman]

clang/lib/Sema/SemaAttr.cpp

1178–1184

Hmm that makes sense. So, pragma optimize(on, "s") would be equivalent to attribute((optimize("-Os")).

Yup, effectively! And in terms of the AST, your pragma can implicitly create an OptimizeAttr with the appropriate arguments; then everything works off the same machinery.

Could you give me a brief description of what I would have to do to implement the attribute? I'm new to this

Absolutely! https://clang.llvm.org/docs/InternalsManual.html#how-to-add-an-attribute has most of the information you need. I'm happy to give you more information if you'd like it, though.

1195

I think if you add an OptimizeAttr, that could be used to carry this information instead of using a dedicated attribute for just the "never optnone" part.

[steplong]

Appreciate the help! It's not clear to me how to go from the strings "Os", "foo1", "foo2", "foo3" to adding "-Os -ffoo1 -ffoo2 -ffoo3" to the compilation line for that function

[aaron.ballman]

In D125723#3550956, @steplong wrote:

Appreciate the help! It's not clear to me how to go from the strings "Os", "foo1", "foo2", "foo3" to adding "-Os -ffoo1 -ffoo2 -ffoo3" to the compilation line for that function

I may be misunderstanding the issue here, but what I had envisioned was that the OptimizeAttr would take either a string or integer argument (I don't know how easy that will be to model in Attr.td, FWIW; it may require adding a new kind of Argument for attributes) and then add a "fake" Argument or use the AdditionalMembers field to add a member that stores the converted "value" of that string or integer in whatever form makes the most sense for the semantics (I was envisioning an enumeration for the various kinds of optimization options, but maybe that doesn't work for the -f arguments?). Then, when doing CodeGen, you can look at the function to see if it has an OptimizeAttr, and if it does, use the fake/additional member to determine what information to lower to IR (or not lower, as the case may be). Does that help get you unstuck somewhat?

Btw, one possibility would be to not support any -f flags initially and only support optimization levels, if that's easier. We can add support for individual flags at a later step but still get utility out of the attribute this way.

[rnk]

In D125723#3553664, @aaron.ballman wrote:

In D125723#3550956, @steplong wrote:

Appreciate the help! It's not clear to me how to go from the strings "Os", "foo1", "foo2", "foo3" to adding "-Os -ffoo1 -ffoo2 -ffoo3" to the compilation line for that function

I may be misunderstanding the issue here, but what I had envisioned was that the OptimizeAttr would take either a string or integer argument (I don't know how easy that will be to model in Attr.td, FWIW; it may require adding a new kind of Argument for attributes) and then add a "fake" Argument or use the AdditionalMembers field to add a member that stores the converted "value" of that string or integer in whatever form makes the most sense for the semantics (I was envisioning an enumeration for the various kinds of optimization options, but maybe that doesn't work for the -f arguments?). Then, when doing CodeGen, you can look at the function to see if it has an OptimizeAttr, and if it does, use the fake/additional member to determine what information to lower to IR (or not lower, as the case may be). Does that help get you unstuck somewhat?

Btw, one possibility would be to not support any -f flags initially and only support optimization levels, if that's easier. We can add support for individual flags at a later step but still get utility out of the attribute this way.

I would really like to limit the scope here to foreclose any future possibility of sending attribute strings through the command line parser. That would expose a big, general API surface, with lots of possibilities for confusing interactions with other features.

I think it will be cleaner to keep the attributes as close as possible to simple booleans, which then correspond relatively directly to the LLVM IR optnone, optsize, etc attributes.

[aaron.ballman]

In D125723#3553705, @rnk wrote:

In D125723#3553664, @aaron.ballman wrote:

In D125723#3550956, @steplong wrote:

Appreciate the help! It's not clear to me how to go from the strings "Os", "foo1", "foo2", "foo3" to adding "-Os -ffoo1 -ffoo2 -ffoo3" to the compilation line for that function

I may be misunderstanding the issue here, but what I had envisioned was that the OptimizeAttr would take either a string or integer argument (I don't know how easy that will be to model in Attr.td, FWIW; it may require adding a new kind of Argument for attributes) and then add a "fake" Argument or use the AdditionalMembers field to add a member that stores the converted "value" of that string or integer in whatever form makes the most sense for the semantics (I was envisioning an enumeration for the various kinds of optimization options, but maybe that doesn't work for the -f arguments?). Then, when doing CodeGen, you can look at the function to see if it has an OptimizeAttr, and if it does, use the fake/additional member to determine what information to lower to IR (or not lower, as the case may be). Does that help get you unstuck somewhat?

Btw, one possibility would be to not support any -f flags initially and only support optimization levels, if that's easier. We can add support for individual flags at a later step but still get utility out of the attribute this way.

I would really like to limit the scope here to foreclose any future possibility of sending attribute strings through the command line parser. That would expose a big, general API surface, with lots of possibilities for confusing interactions with other features.

Agreed.

I think it will be cleaner to keep the attributes as close as possible to simple booleans, which then correspond relatively directly to the LLVM IR optnone, optsize, etc attributes.

Also agreed. One thing I was double-checking and am 99% sure of is that we basically can't support those -f options in Clang anyway and so we likely just want to eat them (silently so we don't cause issues when cross compiling with GCC?). Clang does not have a mapping between -O flags and -f flags in the same way that GCC does. Also, it's not clear to me that there's always a way to specify the -f flag effects for only a single function. Given that GCC documents the optimize attribute as "The optimize attribute should be used for debugging purposes only. It is not suitable in production code.", I think it's reasonable just to punt on -f flags entirely.

[steplong]

• Parse the string in the pragma
• Some of the tests should fail. I'll fix them up once D126984 looks good

[steplong]

clang/include/clang/Sema/Sema.h

768

This might be confusing. ModifyFnAttributeMSPragmaOptimize() adds optnone to functions if MSPragmaOptimizeIsOn is false and MSPragmaOptimizeValidParams is empty.

[xbolva00]

In D125723#3547789, @rnk wrote:

I think it's fine to implement this, but I wanted to share some of the motivation for the current state of things. In our experience, the majority of uses of pragma optimize were to work around MSVC compiler bugs. So, instead of honoring them, we felt it was best to ignore them with a warning (-Wignored-pragma-optimize). Of course, that warning is typically disabled in build systems of large projects, so our current behavior can still surprise users. Implementing the feature is probably the most predictable and least surprising thing Clang can do.

Something we can't easily support for either GCC or MSVC attribute is enabling optimizations for some functions in an -O0 build. Maybe it's now possible to set up a full pass pipeline after semantic analysis is complete, but it's not straightforward. You should consider what to do with that corner case.

What is a real motivation to have it in Clang as well? As you said, for MSVC, to work around bugs. You may say optimise(-Oz), but we have attribute minsize for that. Just to match MSVC?

As shown https://reviews.llvm.org/D126984, things work a bit different in Clang vs MSVC/GCC so to justify this pragma, optimise attribute (and related (huge, imho) rework to move other attributes under OptimiseAttr, maybe rework of LLVM attributes), I would like to hear quite strong motivating words.

[xbolva00]

I would like to hear quite strong motivating words.

From https://reviews.llvm.org/D127565, Aaron mentioned "we have > 400 semantic attributes already and the support matrix for their combinations is pretty bad. ". Yes, this is a real good motivation why to rework these things.

[steplong]

• Change logic to only handle empty optimization list
• Fixup doc and tests

[aaron.ballman]

clang/docs/ReleaseNotes.rst

301

clang/include/clang/Sema/Sema.h

761–762

I think we can get rid of this since it's unused now.

clang/lib/Sema/SemaAttr.cpp

1142

I think this should also be removed, as we never assign anything into that variable anyway.

1144–1149

[steplong]

• Fix docs
• Clean up code

[aaron.ballman]

LGTM! But given the number of people who've expressed opinions on this, please wait a day or two before landing in case other reviewers have additional feedback.

[xbolva00]

clang/docs/ReleaseNotes.rst

304

So to sum up.. #pragma optimize("", off) disables all optimizations, right?

#pragma optimize("", on) enables all, right? is no op if compiled with -O0?

Other than this release note, looks like there is no way to specifiy own documentation for pragmas? :/

Especially if there are differences between MSVC and current implementation...

[xbolva00]

In D125723#3585726, @aaron.ballman wrote:

LGTM! But given the number of people who've expressed opinions on this, please wait a day or two before landing in case other reviewers have additional feedback.

In term of code, implementation ok, some concerns related to documentation added.

[aaron.ballman]

In D125723#3585788, @xbolva00 wrote:

In D125723#3585726, @aaron.ballman wrote:

LGTM! But given the number of people who've expressed opinions on this, please wait a day or two before landing in case other reviewers have additional feedback.

In term of code, implementation ok, some concerns related to documentation added.

Good call! We could add documentation for this pragma to https://github.com/llvm/llvm-project/blob/main/clang/docs/LanguageExtensions.rst and document the differences there.

[xbolva00]

clang/docs/ReleaseNotes.rst

304

Plus, maybe we should emit a warning if #pragma optimize("", on) is used with -O0?

[steplong]

clang/docs/ReleaseNotes.rst

304

Hmm, MSVC says When you use the on parameter, it resets the optimizations to the ones that you specified using the /O compiler option., so I don't think we need to emit a warning. I think we differ from MSVC for off. For MSVC, off turns off frame pointers, "s", and "t". We are just mapping that to optnone.

[steplong]

• Add documentation on difference btwn MSVC and Clang's implementation of pragma optimize

[xbolva00]

Thanks

clang/docs/LanguageExtensions.rst

3800 ↗

(On Diff #437260)

as the clang pragma?

[steplong]

clang/docs/LanguageExtensions.rst

3800 ↗

(On Diff #437260)

The clang pragma #pragma clang optimize off. It's mentioned in this section, but hmm maybe I can make it more clear

[steplong]

• Fix docs

[steplong]

Hi, I'll land this on Friday. Let me know if there are any comments. Also, I'm not sure how to rebase it on top of main. At the moment, it's still on top of D126984 (i.e. stack has one open)

[aaron.ballman]

clang/docs/LanguageExtensions.rst

3798 ↗

(On Diff #437280)

3799–3800 ↗

(On Diff #437280)

Would this be more clear?

[steplong]

• Fix up docs

[aaron.ballman]

LGTM!

[steplong]

• Run pre-merge checks again after abandoning dependent patch D126984

clang/docs/ReleaseNotes.rst

 New Pragmas in Clang
 --------------------
 - Added support for MSVC's ``#pragma function``, which tells the compiler to
   generate calls to functions listed in the pragma instead of using the
   builtins.
 - Added support for MSVC's ``#pragma alloc_text``. The pragma names the code
   section functions are placed in. The pragma only applies to functions with
   C linkage.
+- Added support for an empty optimization list for MSVC's ``#pragma optimize``.
+  The pragma takes a list of optimizations to turn on or off which applies to
+  all functions following the pragma. At the moment, only an empty list is
+  supported.
 
 - ...
 
 Attribute Changes in Clang
 --------------------------
 
 - Added support for parameter pack expansion in ``clang::annotate``.
 

clang/include/clang/Basic/DiagnosticParseKinds.td

def warn_pragma_invalid_action : Warning<
   InGroup<IgnoredPragmas>;
 def warn_pragma_pack_malformed : Warning<
   "expected integer or identifier in '#pragma pack' - ignored">,
   InGroup<IgnoredPragmas>;
 // - #pragma intrinsic
 def warn_pragma_intrinsic_builtin : Warning<
   "%0 is not a recognized builtin%select{|; consider including <intrin.h> to access non-builtin intrinsics}1">,
   InGroup<IgnoredPragmaIntrinsic>;
-// - #pragma optimize
-def warn_pragma_optimize : Warning<
-  "'#pragma optimize' is not supported">,
-  InGroup<IgnoredPragmaOptimize>;
 // - #pragma unused
 def warn_pragma_unused_expected_var : Warning<
   "expected '#pragma unused' argument to be a variable name">,
   InGroup<IgnoredPragmas>;
 // - #pragma init_seg
 def warn_pragma_init_seg_unsupported_target : Warning<
   "'#pragma init_seg' is only supported when targeting a "
   "Microsoft environment">,

clang/include/clang/Parse/Parser.h

private:
   bool HandlePragmaMSSegment(StringRef PragmaName,
                              SourceLocation PragmaLocation);
   bool HandlePragmaMSInitSeg(StringRef PragmaName,
                              SourceLocation PragmaLocation);
   bool HandlePragmaMSFunction(StringRef PragmaName,
                               SourceLocation PragmaLocation);
   bool HandlePragmaMSAllocText(StringRef PragmaName,
                                SourceLocation PragmaLocation);
+  bool HandlePragmaMSOptimize(StringRef PragmaName,
+                              SourceLocation PragmaLocation);
 
   /// Handle the annotation token produced for
   /// #pragma align...
   void HandlePragmaAlign();
 
   /// Handle the annotation token produced for
   /// #pragma clang __debug dump...
   void HandlePragmaDump();

clang/include/clang/Sema/Sema.h

public:
   /// #pragma attribute stack.
   const Decl *PragmaAttributeCurrentTargetDecl;
 
   /// 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;
 
+  /// 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
+  /// options are honored when `#pragma optimize("", on)`.
+  /// (i.e. `ModifyFnAttributeMSPragmaOptimze()` does nothing)
+  bool MSPragmaOptimizeIsOn = true;
+
   /// Set of no-builtin functions listed by \#pragma function.
   llvm::SmallSetVector<StringRef, 4> MSFunctionNoBuiltins;
 
   /// Flag indicating if Sema is building a recovery call expression.
   ///
   /// This flag is used to avoid building recovery call expressions
   /// if Sema is already doing so, which would cause infinite recursions.
   bool IsBuildingRecoveryCallExpr;
 

   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
   void DiagnoseUnterminatedPragmaAttribute();
 
   /// Called on well formed \#pragma clang optimize.
   void ActOnPragmaOptimize(bool On, SourceLocation PragmaLoc);
 
+  /// #pragma optimize("[optimization-list]", on | off).
+  void ActOnPragmaMSOptimize(SourceLocation Loc, bool IsOn);
+
   /// Call on well formed \#pragma function.
   void
   ActOnPragmaMSFunction(SourceLocation Loc,
                         const llvm::SmallVectorImpl<StringRef> &NoBuiltins);
 
   /// 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() const {

   /// attribute section to the function.
   void AddSectionMSAllocText(FunctionDecl *FD);
 
   /// 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);
 
+  /// 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.
+  void ModifyFnAttributesMSPragmaOptimize(FunctionDecl *FD);
+
   /// Only called on function definitions; if there is a pragma in scope
   /// with the effect of a range-based no_builtin, consider marking the function
   /// with attribute no_builtin.
   void AddImplicitMSFunctionNoBuiltinAttr(FunctionDecl *FD);
 
   /// AddAlignedAttr - Adds an aligned attribute to a particular declaration.
   void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
                       bool IsPackExpansion);

clang/lib/Parse/ParsePragma.cpp

 };
 
 struct PragmaMSIntrinsicHandler : public PragmaHandler {
   PragmaMSIntrinsicHandler() : PragmaHandler("intrinsic") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
                     Token &FirstToken) override;
 };
 
-struct PragmaMSOptimizeHandler : public PragmaHandler {
-  PragmaMSOptimizeHandler() : PragmaHandler("optimize") {}
-  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
-                    Token &FirstToken) override;
-};
-
 // "\#pragma fenv_access (on)".
 struct PragmaMSFenvAccessHandler : public PragmaHandler {
   PragmaMSFenvAccessHandler() : PragmaHandler("fenv_access") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
                     Token &FirstToken) override {
     StringRef PragmaName = FirstToken.getIdentifierInfo()->getName();
     if (!PP.getTargetInfo().hasStrictFP() && !PP.getLangOpts().ExpStrictFP) {
       PP.Diag(FirstToken.getLocation(), diag::warn_pragma_fp_ignored)
if (getLangOpts().MicrosoftExt) {
     MSCodeSeg = std::make_unique<PragmaMSPragma>("code_seg");
     PP.AddPragmaHandler(MSCodeSeg.get());
     MSSection = std::make_unique<PragmaMSPragma>("section");
     PP.AddPragmaHandler(MSSection.get());
     MSFunction = std::make_unique<PragmaMSPragma>("function");
     PP.AddPragmaHandler(MSFunction.get());
     MSAllocText = std::make_unique<PragmaMSPragma>("alloc_text");
     PP.AddPragmaHandler(MSAllocText.get());
+    MSOptimize = std::make_unique<PragmaMSPragma>("optimize");
+    PP.AddPragmaHandler(MSOptimize.get());
     MSRuntimeChecks = std::make_unique<PragmaMSRuntimeChecksHandler>();
     PP.AddPragmaHandler(MSRuntimeChecks.get());
     MSIntrinsic = std::make_unique<PragmaMSIntrinsicHandler>();
     PP.AddPragmaHandler(MSIntrinsic.get());
-    MSOptimize = std::make_unique<PragmaMSOptimizeHandler>();
-    PP.AddPragmaHandler(MSOptimize.get());
     MSFenvAccess = std::make_unique<PragmaMSFenvAccessHandler>();
     PP.AddPragmaHandler(MSFenvAccess.get());
   }
 
   if (getLangOpts().CUDA) {
     CUDAForceHostDeviceHandler =
         std::make_unique<PragmaForceCUDAHostDeviceHandler>(Actions);
     PP.AddPragmaHandler("clang", CUDAForceHostDeviceHandler.get());
PragmaHandler Handler =
       llvm::StringSwitch<PragmaHandler>(PragmaName)
           .Case("data_seg", &Parser::HandlePragmaMSSegment)
           .Case("bss_seg", &Parser::HandlePragmaMSSegment)
           .Case("const_seg", &Parser::HandlePragmaMSSegment)
           .Case("code_seg", &Parser::HandlePragmaMSSegment)
           .Case("section", &Parser::HandlePragmaMSSection)
           .Case("init_seg", &Parser::HandlePragmaMSInitSeg)
           .Case("function", &Parser::HandlePragmaMSFunction)
-          .Case("alloc_text", &Parser::HandlePragmaMSAllocText);
+          .Case("alloc_text", &Parser::HandlePragmaMSAllocText)
+          .Case("optimize", &Parser::HandlePragmaMSOptimize);
 
   if (!(this->*Handler)(PragmaName, PragmaLocation)) {
     // Pragma handling failed, and has been diagnosed.  Slurp up the tokens
     // until eof (really end of line) to prevent follow-on errors.
     while (Tok.isNot(tok::eof))
       PP.Lex(Tok);
     PP.Lex(Tok);
   }
if (ExpectAndConsume(tok::r_paren, diag::warn_pragma_expected_rparen,
                        PragmaName))
     return false;
 
   Actions.ActOnPragmaMSFunction(FirstTok.getLocation(), NoBuiltins);
   return true;
 }
 
 // #pragma optimize("gsty", on|off)
-void PragmaMSOptimizeHandler::HandlePragma(Preprocessor &PP,
-                                           PragmaIntroducer Introducer,
-                                           Token &Tok) {
-  SourceLocation StartLoc = Tok.getLocation();
-  PP.Lex(Tok);
-
-  if (Tok.isNot(tok::l_paren)) {
-    PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen) << "optimize";
-    return;
-  }
-  PP.Lex(Tok);
+bool Parser::HandlePragmaMSOptimize(StringRef PragmaName,
+                                    SourceLocation PragmaLocation) {
+  Token FirstTok = Tok;
+  if (ExpectAndConsume(tok::l_paren, diag::warn_pragma_expected_lparen,
+                       PragmaName))
+    return false;
 
   if (Tok.isNot(tok::string_literal)) {
-    PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_string) << "optimize";
-    return;
+    PP.Diag(PragmaLocation, diag::warn_pragma_expected_string) << PragmaName;
+    return false;
   }
-  // We could syntax check the string but it's probably not worth the effort.
-  PP.Lex(Tok);
-
-  if (Tok.isNot(tok::comma)) {
-    PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_comma) << "optimize";
-    return;
+  ExprResult StringResult = ParseStringLiteralExpression();
+  if (StringResult.isInvalid())
+    return false; // Already diagnosed.
+  StringLiteral *OptimizationList = cast<StringLiteral>(StringResult.get());
+  if (OptimizationList->getCharByteWidth() != 1) {
+    PP.Diag(PragmaLocation, diag::warn_pragma_expected_non_wide_string)
+        << PragmaName;
+    return false;
   }
-  PP.Lex(Tok);
 
-  if (Tok.is(tok::eod) || Tok.is(tok::r_paren)) {
-    PP.Diag(Tok.getLocation(), diag::warn_pragma_missing_argument)
-        << "optimize" << /*Expected=*/true << "'on' or 'off'";
-    return;
+  if (ExpectAndConsume(tok::comma, diag::warn_pragma_expected_comma,
+                       PragmaName))
+    return false;
+
+  if (Tok.is(tok::eof) || Tok.is(tok::r_paren)) {
+    PP.Diag(PragmaLocation, diag::warn_pragma_missing_argument)
+        << PragmaName << /*Expected=*/true << "'on' or 'off'";
+    return false;
   }
   IdentifierInfo *II = Tok.getIdentifierInfo();
   if (!II || (!II->isStr("on") && !II->isStr("off"))) {
-    PP.Diag(Tok.getLocation(), diag::warn_pragma_invalid_argument)
-        << PP.getSpelling(Tok) << "optimize" << /*Expected=*/true
+    PP.Diag(PragmaLocation, diag::warn_pragma_invalid_argument)
+        << PP.getSpelling(Tok) << PragmaName << /*Expected=*/true
         << "'on' or 'off'";
-    return;
+    return false;
   }
+  bool IsOn = II->isStr("on");
   PP.Lex(Tok);
 
-  if (Tok.isNot(tok::r_paren)) {
-    PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_rparen) << "optimize";
-    return;
-  }
-  PP.Lex(Tok);
+  if (ExpectAndConsume(tok::r_paren, diag::warn_pragma_expected_rparen,
+                       PragmaName))
+    return false;
 
-  if (Tok.isNot(tok::eod)) {
-    PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
-        << "optimize";
-    return;
+  // TODO: Add support for "sgty"
+  if (!OptimizationList->getString().empty()) {
+    PP.Diag(PragmaLocation, diag::warn_pragma_invalid_argument)
+        << OptimizationList->getString() << PragmaName << /*Expected=*/true
+        << "\"\"";
+    return false;
   }
-  PP.Diag(StartLoc, diag::warn_pragma_optimize);
+
+  if (ExpectAndConsume(tok::eof, diag::warn_pragma_extra_tokens_at_eol,
+                       PragmaName))
+    return false;
+
+  Actions.ActOnPragmaMSOptimize(FirstTok.getLocation(), IsOn);
+  return true;
 }
 
 void PragmaForceCUDAHostDeviceHandler::HandlePragma(
     Preprocessor &PP, PragmaIntroducer Introducer, Token &Tok) {
   Token FirstTok = Tok;
 
   PP.Lex(Tok);
   IdentifierInfo *Info = Tok.getIdentifierInfo();

clang/lib/Sema/SemaAttr.cpp

 
 void Sema::ActOnPragmaOptimize(bool On, SourceLocation PragmaLoc) {
   if(On)
     OptimizeOffPragmaLocation = SourceLocation();
   else
     OptimizeOffPragmaLocation = PragmaLoc;
 }
 
+void Sema::ActOnPragmaMSOptimize(SourceLocation Loc, bool IsOn) {
+  if (!CurContext->getRedeclContext()->isFileContext()) {
+    Diag(Loc, diag::err_pragma_expected_file_scope) << "optimize";
+    return;
+  }
+
+  MSPragmaOptimizeIsOn = IsOn;
+}
+
 void Sema::ActOnPragmaMSFunction(
     SourceLocation Loc, const llvm::SmallVectorImpl<StringRef> &NoBuiltins) {
   if (!CurContext->getRedeclContext()->isFileContext()) {
     Diag(Loc, diag::err_pragma_expected_file_scope) << "function";
     return;
   }
 
   MSFunctionNoBuiltins.insert(NoBuiltins.begin(), NoBuiltins.end());
 }
if (It != FunctionToSectionMap.end()) {
     SourceLocation Loc;
     std::tie(Section, Loc) = It->second;
 
     if (!FD->hasAttr<SectionAttr>())
       FD->addAttr(SectionAttr::CreateImplicit(Context, Section));
   }
 }
 
+void Sema::ModifyFnAttributesMSPragmaOptimize(FunctionDecl *FD) {
+  // Don't modify the function attributes if it's "on". "on" resets the
+  // optimizations to the ones listed on the command line
+  if (!MSPragmaOptimizeIsOn)
+    AddOptnoneAttributeIfNoConflicts(FD, FD->getBeginLoc());
+}
+
 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));

 enum : unsigned { NoVisibility = ~0U };
 
 void Sema::AddPushedVisibilityAttribute(Decl *D) {
   if (!VisContext)
     return;
 
   NamedDecl *ND = dyn_cast<NamedDecl>(D);
   if (ND && ND->getExplicitVisibility(NamedDecl::VisibilityForValue))
     return;
 
   VisStack *Stack = static_cast<VisStack*>(VisContext);
   unsigned rawType = Stack->back().first;
   if (rawType == NoVisibility) return;
 
   VisibilityAttr::VisibilityType type
     = (VisibilityAttr::VisibilityType) rawType;
   SourceLocation loc = Stack->back().second;
 
   D->addAttr(VisibilityAttr::CreateImplicit(Context, type, loc));
 }
 
 /// FreeVisContext - Deallocate and null out VisContext.
 void Sema::FreeVisContext() {
   delete static_cast<VisStack*>(VisContext);
   VisContext = nullptr;
 }
 
 static void PushPragmaVisibility(Sema &S, unsigned type, SourceLocation loc) {
   // Put visibility on stack.
   if (!S.VisContext)
     S.VisContext = new VisStack;
 
   VisStack *Stack = static_cast<VisStack*>(S.VisContext);
   Stack->push_back(std::make_pair(type, loc));

clang/lib/Sema/SemaDecl.cpp

Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
   AddCFAuditedAttribute(NewFD);
 
   // If this is a function definition, check if we have to apply any
   // attributes (i.e. optnone and no_builtin) due to a pragma.
   if (D.isFunctionDefinition()) {
     AddRangeBasedOptnone(NewFD);
     AddImplicitMSFunctionNoBuiltinAttr(NewFD);
     AddSectionMSAllocText(NewFD);
+    ModifyFnAttributesMSPragmaOptimize(NewFD);
   }
 
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
   if (NewFD->isFirstDecl() && !NewFD->isInvalidDecl() &&
       isIncompleteDeclExternC(*this, NewFD))
     RegisterLocallyScopedExternCDecl(NewFD, S);
 

clang/test/CodeGen/pragma-msvc-optimize.c

+// RUN: %clang_cc1 -O2 -emit-llvm -fms-extensions -o - %s | FileCheck %s
+
+#pragma optimize("", off)
+
+// CHECK: define{{.*}} void @f0(){{.*}} #[[OPTNONE:[0-9]+]]
+void f0() {}
+
+// CHECK: define{{.*}} void @f1(){{.*}} #[[OPTNONE]]
+void f1() {}
+
+#pragma optimize("", on)
+
+// CHECK: define{{.*}} void @f2(){{.*}} #[[NO_OPTNONE:[0-9]+]]
+void f2() {}
+
+// CHECK: define{{.*}} void @f3(){{.*}} #[[NO_OPTNONE]]
+void f3() {}
+
+// CHECK:         attributes #[[OPTNONE]] = {{{.*}}optnone{{.*}}}
+// CHECK-NOT:     attributes #[[NO_OPTNONE]] = {{{.*}}optnone{{.*}}}

clang/test/Preprocessor/pragma_microsoft.c

 }
 
 #pragma optimize          // expected-warning{{missing '(' after '#pragma optimize'}}
 #pragma optimize(         // expected-warning{{expected string literal in '#pragma optimize'}}
 #pragma optimize(a        // expected-warning{{expected string literal in '#pragma optimize'}}
 #pragma optimize("g"      // expected-warning{{expected ',' in '#pragma optimize'}}
 #pragma optimize("g",     // expected-warning{{missing argument to '#pragma optimize'; expected 'on' or 'off'}}
 #pragma optimize("g",xyz  // expected-warning{{unexpected argument 'xyz' to '#pragma optimize'; expected 'on' or 'off'}}
-#pragma optimize("g",on)  // expected-warning{{#pragma optimize' is not supported}}
+#pragma optimize("g",on)  // expected-warning{{unexpected argument 'g' to '#pragma optimize'; expected ""}}
+#pragma optimize("",on)  // no-warning
+#pragma optimize("", on) asdf // expected-warning{{extra tokens at end of '#pragma optimize'}}
+
+void pragma_optimize_foo() {
+#pragma optimize("", on) // expected-error {{'#pragma optimize' can only appear at file scope}}
+}
 
 #pragma execution_character_set                 // expected-warning {{expected '('}}
 #pragma execution_character_set(                // expected-warning {{expected 'push' or 'pop'}}
 #pragma execution_character_set()               // expected-warning {{expected 'push' or 'pop'}}
 #pragma execution_character_set(asdf            // expected-warning {{expected 'push' or 'pop'}}
 #pragma execution_character_set(asdf)           // expected-warning {{expected 'push' or 'pop'}}
 #pragma execution_character_set(push            // expected-warning {{expected ')'}}
 #pragma execution_character_set(pop,)           // expected-warning {{expected ')'}}