[PATCH] D125723: [MSVC] Add support for MSVC pragma optimize

[Stephen Long via Phabricator via cfe-commits]

steplong added inline comments.


================
Comment at: clang/lib/Sema/SemaAttr.cpp:1207-1213
+  FD->addAttr(FramePointerAttr::CreateImplicit(Context, Kind));
+}
+
+void Sema::AddOptsize(FunctionDecl *FD, SourceLocation Loc) {
+  FD->dropAttr<OptimizeSizeAttr>();
+  OptimizeSizeAttr::Kind Kind = OptimizeSizeAttr::On;
+  FD->addAttr(OptimizeSizeAttr::CreateImplicit(Context, Kind));
----------------
aaron.ballman wrote:
> 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.
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


================
Comment at: clang/lib/Sema/SemaAttr.cpp:1224
+  FD->dropAttr<OptimizeNoneAttr>();
+  FD->addAttr(NeverOptimizeNoneAttr::CreateImplicit(Context));
+}
----------------
aaron.ballman wrote:
> Can you explain why this is needed?
I added this attr, so we can remove the attr optnone even if -O0 was passed on the cmdline


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125723/new/

https://reviews.llvm.org/D125723