Start using optimization (-O0/-O2/-O3/-Os) and debug (-g) flags from CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE} #33388

kubamracek · 2020-08-10T19:59:11Z

No description provided.

compnerd

This is definitely a pretty big improvement.

cmake/modules/AddSwift.cmake

kubamracek · 2020-08-11T16:08:56Z

I want to point out that this changes one of the optimization levels. The CMake defaults are:

CMAKE_CXX_FLAGS_DEBUG = -g
CMAKE_CXX_FLAGS_MINSIZEREL = -Os -DNDEBUG
CMAKE_CXX_FLAGS_RELEASE = -O3 -DNDEBUG
CMAKE_CXX_FLAGS_RELWITHDEBINFO = -O2 -g -DNDEBUG

While currently Swift forces -O2 on Release. So this will be a change to -O3 here.

…CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE}

kubamracek · 2020-08-12T16:13:03Z

@swift-ci please test

swift-ci · 2020-08-12T16:18:03Z

Build failed
Swift Test Linux Platform
Git Sha - cb5de39c4715e9b74efed0486f80ca68b1026a41

swift-ci · 2020-08-12T16:18:09Z

Build failed
Swift Test OS X Platform
Git Sha - cb5de39c4715e9b74efed0486f80ca68b1026a41

kubamracek · 2020-08-12T21:05:19Z

@swift-ci please test macOS platform

kubamracek · 2020-08-13T00:50:02Z

@swift-ci please test macOS platform

kubamracek · 2020-08-13T15:18:24Z

@gottesmm @airspeedswift @aschwaighofer Any more feedback here? Any concerns about the switch to -O3 for Release (not for RelWithDebInfo though)?

airspeedswift · 2020-08-13T15:20:44Z

We really need to do a benchmark run and see the impact on that.

aschwaighofer · 2020-08-13T15:23:38Z

Isn't -O3 clang's benchmarking mode? By that I mean the kitchen sink that is used when compiling 'insert your favorite commercial benchmark here' . And -Os/-O2 is meant to be more sane.

What are the code size impacts of using -O3 over -Os on the c++ part of the standard library?

I seemed to remember the loop vectorizer for example inserts speculative code aggressively in the highest opt mode.

kubamracek · 2020-08-13T15:47:52Z

Okay, so it sounds like there are concerns. @compnerd how about I do something like:

set(CMAKE_CXX_FLAGS_RELEASE "-O2 -DNDEBUG")

to turn this PR into a NFC, and we can discuss separately whether we should switch to the CMake's default?

kubamracek · 2020-08-13T16:12:37Z

Ok, this is now a NFC change only.

kubamracek · 2020-08-13T16:12:44Z

@swift-ci please test

kubamracek · 2020-08-13T18:01:53Z

@swift-ci please test

kubamracek · 2020-08-13T22:25:02Z

@swift-ci please test Windows platform

kubamracek · 2020-08-13T23:20:55Z

@swift-ci please test Windows platform

kubamracek · 2020-08-14T00:58:36Z

Hm, the Windows bot failure looks like a real regression: https://ci-external.swift.org/job/swift-PR-windows/6268/console. @compnerd any ideas what's causing it?

kubamracek · 2020-08-16T18:41:54Z

@swift-ci please test Windows platform

kubamracek · 2020-08-16T18:51:59Z

@swift-ci please test Windows platform

kubamracek · 2020-08-17T03:25:12Z

@swift-ci please test

swift-ci · 2020-08-17T03:28:05Z

Build failed
Swift Test Linux Platform
Git Sha - cd2c471

swift-ci · 2020-08-17T03:28:11Z

Build failed
Swift Test OS X Platform
Git Sha - cd2c471

compnerd · 2020-08-17T18:01:13Z

CMakeLists.txt

@@ -629,6 +629,19 @@ if("${CMAKE_C_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQU
  set(SWIFT_COMPILER_IS_MSVC_LIKE TRUE)
 endif()

+if(NOT SWIFT_COMPILER_IS_MSVC_LIKE)
+  # Avoid CMake's default -O3 for Release builds.
+  set(CMAKE_CXX_FLAGS_RELEASE "-O2 -DNDEBUG")


Please don't set -DNDEBUG, that overrides the behaviour of LLVM_ENABLE_ASSERTIONS. I usually build with assertions intentionally.

It is the default in CMAKE_CXX_FLAGS_RELEASE, though. I though it would be better to keep it that way, because we're explicitly checking LLVM_ENABLE_ASSERTIONS and passing -UNDEBUG or -DNDEBUG after we include CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE}.

cmake/modules/AddSwift.cmake

kubamracek · 2020-08-17T20:44:07Z

@swift-ci please test

swift-ci · 2020-08-17T20:47:07Z

Build failed
Swift Test Linux Platform
Git Sha - 5066bea

swift-ci · 2020-08-17T20:47:50Z

Build failed
Swift Test OS X Platform
Git Sha - 5066bea

gottesmm · 2020-08-18T05:03:00Z

@kubamracek leaving swift at -O2 is the correct thing to do here! Thanks for keeping it like that!

Set those at the value pre swiftlang#33388 -- this would avoid regressions of disk space usage (in particular around building Swift LTO) Addresses rdar://68091272

Set those at the value pre #33388 -- this would avoid regressions of disk space usage (in particular around building Swift LTO) Addresses rdar://68091272

…gs from CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE} (swiftlang#33388)" This reverts commit 28a2826.

…gs from CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE} (#33388)" (#33924)

kubamracek requested review from compnerd and gottesmm August 10, 2020 19:59

kubamracek mentioned this pull request Aug 10, 2020

Add a 'standalone_minimal' preset to build a minimal, static, OS independent, self-contained binaries of stdlib. #33286

Merged

compnerd reviewed Aug 10, 2020

View reviewed changes

cmake/modules/AddSwift.cmake Show resolved Hide resolved

cmake/modules/AddSwift.cmake Outdated Show resolved Hide resolved

kubamracek requested review from jckarter, shahmishal, devincoughlin, aschwaighofer and airspeedswift August 11, 2020 15:51

kubamracek added 2 commits August 12, 2020 09:11

Start using optimization (-O0/-O2/-O3/-Os) and debug (-g) flags from …

6612d8c

…CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE}

Use -gline-tables-only from CMAKE_CXX_FLAGS_... too

7ccbc75

kubamracek force-pushed the flags_from_build_type branch from d515898 to 7ccbc75 Compare August 12, 2020 16:12

Avoid CMake's default -O3 for Release builds

cd2c471

Set CMAKE_CXX_FLAGS_RELEASE only for non-MSVC compilers

5066bea

compnerd reviewed Aug 17, 2020

View reviewed changes

kubamracek added 3 commits August 17, 2020 13:13

Add explaning comments

108708f

Tweak comment and remove unnecessary -Os/-O2 appending

7e50def

Set CMAKE_CXX_FLAGS_RELEASE to just "-O2", add explaining comment

8ff768f

kubamracek merged commit 28a2826 into swiftlang:master Aug 18, 2020

kubamracek deleted the flags_from_build_type branch August 18, 2020 02:26

edymtt mentioned this pull request Sep 1, 2020

[build] Reinstate CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE} for LLVM #33745

Merged

kubamracek added a commit to kubamracek/swift that referenced this pull request Sep 12, 2020

Revert "Start using optimization (-O0/-O2/-O3/-Os) and debug (-g) fla…

2c59f10

…gs from CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE} (swiftlang#33388)" This reverts commit 28a2826.

kubamracek added a commit that referenced this pull request Sep 12, 2020

Revert "Start using optimization (-O0/-O2/-O3/-Os) and debug (-g) fla…

d9d929a

…gs from CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE} (#33388)" (#33924)

Start using optimization (-O0/-O2/-O3/-Os) and debug (-g) flags from CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE} #33388

Start using optimization (-O0/-O2/-O3/-Os) and debug (-g) flags from CMAKE_CXX_FLAGS_${CFLAGS_BUILD_TYPE} #33388

Uh oh!

Conversation

kubamracek commented Aug 10, 2020

Uh oh!

compnerd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kubamracek commented Aug 11, 2020

Uh oh!

kubamracek commented Aug 12, 2020

Uh oh!

swift-ci commented Aug 12, 2020

Uh oh!

swift-ci commented Aug 12, 2020

Uh oh!

kubamracek commented Aug 12, 2020

Uh oh!

kubamracek commented Aug 13, 2020

Uh oh!

kubamracek commented Aug 13, 2020

Uh oh!

airspeedswift commented Aug 13, 2020

Uh oh!

aschwaighofer commented Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kubamracek commented Aug 13, 2020

Uh oh!

kubamracek commented Aug 13, 2020

Uh oh!

kubamracek commented Aug 13, 2020

Uh oh!

kubamracek commented Aug 13, 2020

Uh oh!

kubamracek commented Aug 13, 2020

Uh oh!

kubamracek commented Aug 13, 2020

Uh oh!

kubamracek commented Aug 14, 2020

Uh oh!

kubamracek commented Aug 16, 2020

Uh oh!

kubamracek commented Aug 16, 2020

Uh oh!

kubamracek commented Aug 17, 2020

Uh oh!

swift-ci commented Aug 17, 2020

Uh oh!

swift-ci commented Aug 17, 2020

Uh oh!

compnerd Aug 17, 2020

Choose a reason for hiding this comment

Uh oh!

kubamracek Aug 17, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kubamracek commented Aug 17, 2020

Uh oh!

swift-ci commented Aug 17, 2020

Uh oh!

swift-ci commented Aug 17, 2020

Uh oh!

gottesmm commented Aug 18, 2020

Uh oh!

Uh oh!

aschwaighofer commented Aug 13, 2020 •

edited

Loading