Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

clang-cl ignores #pragma optimize("", off), continues optimizing #38119

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR39146
Status NEW
Importance P enhancement
Reported by Bruce Dawson (brucedawson@chromium.org)
Reported on 2018-10-01 15:03:07 -0700
Last modified on 2018-10-02 10:00:10 -0700
Version trunk
Hardware PC Windows NT
CC dblaikie@gmail.com, dgregor@apple.com, llvm-bugs@lists.llvm.org, paul_robinson@playstation.sony.com, rnk@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

A handy technique when working on optimized builds is to disable optimizations for a single source file or function by adding this to the source:

pragma optimize("", off)

With clang-cl this seems to be silently ignored - functions are still inlined and become undebuggable. The desired effect can be obtained with this:

pragma clang optimize off

However, if clang-cl is trying to maintain the semantics of cl.exe then it needs to support #pragma optimize - and __pragma(optimize... - to avoid the need for source-code changes.

Chrome's repo has about 35 instances of #pragma optimize("", off), and a couple of instances of #pragma optimize("g", off). Some of these are bogus and should be removed, but the compiler should still support the construct, or warn if it is being ignored (maybe warn on unsupported variants).

Quuxplusone commented 5 years ago
We actually have -Wignored-pragma-optimize for this, and it's disabled in
Chromium:
https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?type=cs&q=Wno-ignored-pragma-optimize&sq=package:chromium&g=0&l=1503
http://crbug.com/505314

Basically, 99% of committed instances of this pragma were to work around MSVC
optimization bugs that LLVM doesn't have. Of course, it's confusing when you
use it interactively for debugging purposes. So, we have the warning, but it's
off because then it would fire in chromium. =/

Do you think we should go ahead and support it anyway?
Quuxplusone commented 5 years ago

We recently got rid of some of the #pragma optimize directives in Chrome and I'd love to get rid of more for the reasons that you mention - the workarounds for VS bugs (crbug.com/804884) or the uses for misguided reasons.

Supporting #pragma optimize will, at the very least, make it easier for old-time VS users to temporarily disable optimizations. But it will also disable optimizations in a few places where it isn't necessary. Overall I think it's probably worth it.

The alternative would be to update MSVC_DISABLE_OPTIMIZE, perhaps with variations for working around compiler bugs versus other purposes, but that would only benefit Chrome. For the larger ecosystem I tend to think that clang-cl should support this pragma.

I filed crbug.com/891075 to track doing some Chromium cleanup.

We should remove -Wno-ignored-pragma-optimize from Chromium code if we don't make this change, to increase the odds that Chromium developers will avoid being confused by this not working.

Quuxplusone commented 5 years ago

I'm actually quite conflicted. Updating MSVC_DISABLE_OPTIMIZE and then cleaning up Chromium's use of it and then enabling the ignored-pragma-optimize warning in Chromium is pretty tempting. Hmmm...