Skip to content

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Oct 10, 2023

This patch adds a configuration of the libc++ test suite that enables optimizations when building the tests. It also adds a new CI configuration to exercise this on a regular basis. This is added in the context of 1, which requires building with optimizations in order to hit the bug.

@ldionne ldionne requested a review from a team as a code owner October 10, 2023 23:44
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 10, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2023

@llvm/pr-subscribers-github-workflow
@llvm/pr-subscribers-libunwind

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This patch adds a configuration of the libc++ test suite that enables optimizations when building the tests. It also adds a new CI configuration to exercise this on a regular basis. This is added in the context of 1, which requires building with optimizations in order to hit the bug.


Full diff: https://github.com/llvm/llvm-project/pull/68753.diff

4 Files Affected:

  • (added) libcxx/cmake/caches/Generic-optimized.cmake (+4)
  • (modified) libcxx/utils/ci/buildkite-pipeline.yml (+18)
  • (modified) libcxx/utils/ci/run-buildbot (+5)
  • (modified) libcxx/utils/libcxx/test/params.py (+27-1)
diff --git a/libcxx/cmake/caches/Generic-optimized.cmake b/libcxx/cmake/caches/Generic-optimized.cmake
new file mode 100644
index 000000000000000..577a5de9f34c539
--- /dev/null
+++ b/libcxx/cmake/caches/Generic-optimized.cmake
@@ -0,0 +1,4 @@
+set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+set(LIBCXX_TEST_PARAMS "optimization=speed" CACHE STRING "")
+set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
+set(LIBUNWIND_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
diff --git a/libcxx/utils/ci/buildkite-pipeline.yml b/libcxx/utils/ci/buildkite-pipeline.yml
index ebfb35eee91e1ed..1b52d994081c46f 100644
--- a/libcxx/utils/ci/buildkite-pipeline.yml
+++ b/libcxx/utils/ci/buildkite-pipeline.yml
@@ -743,6 +743,24 @@ steps:
           limit: 2
     timeout_in_minutes: 120
 
+  - label: "Optimized build and test suite"
+    command: "libcxx/utils/ci/run-buildbot generic-optimized"
+    artifact_paths:
+      - "**/test-results.xml"
+      - "**/*.abilist"
+    env:
+        CC: "clang-${LLVM_HEAD_VERSION}"
+        CXX: "clang++-${LLVM_HEAD_VERSION}"
+        ENABLE_CLANG_TIDY: "On"
+    agents:
+      queue: "libcxx-builders"
+      os: "linux"
+    retry:
+      automatic:
+        - exit_status: -1  # Agent was lost
+          limit: 2
+    timeout_in_minutes: 120
+
   # Other non-testing CI jobs
   - label: "Benchmarks"
     command: "libcxx/utils/ci/run-buildbot benchmarks"
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index a71318123db3b12..18243b44a3d745c 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -479,6 +479,11 @@ generic-abi-unstable)
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-abi-unstable.cmake"
     check-runtimes
 ;;
+generic-optimized)
+    clean
+    generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-optimized.cmake"
+    check-runtimes
+;;
 apple-system)
     clean
 
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index 456794b9b1cce95..9452f179aea3fec 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -11,7 +11,7 @@
 from pathlib import Path
 
 from libcxx.test.dsl import *
-from libcxx.test.features import _isMSVC
+from libcxx.test.features import _isClang, _isAppleClang, _isGCC, _isMSVC
 
 
 _warningFlags = [
@@ -90,6 +90,21 @@ def getStdFlag(cfg, std):
         return "-std=" + fallbacks[std]
     return None
 
+def getSpeedOptimizationFlag(cfg):
+    if _isClang(cfg) or _isAppleClang(cfg) or _isGCC(cfg):
+        return "-O3"
+    elif _isMSVC(cfg):
+        return "/O2"
+    else:
+        raise RuntimeError("Can't figure out what compiler is used in the configuration")
+
+def getSizeOptimizationFlag(cfg):
+    if _isClang(cfg) or _isAppleClang(cfg) or _isGCC(cfg):
+        return "-Os"
+    elif _isMSVC(cfg):
+        return "/O1"
+    else:
+        raise RuntimeError("Can't figure out what compiler is used in the configuration")
 
 # fmt: off
 DEFAULT_PARAMETERS = [
@@ -121,6 +136,17 @@ def getStdFlag(cfg, std):
             AddCompileFlag(lambda cfg: getStdFlag(cfg, std)),
         ],
     ),
+    Parameter(
+        name="optimization",
+        choices=["none", "speed", "size"],
+        type=str,
+        help="The version of the standard to compile the test suite with.",
+        default="none",
+        actions=lambda opt: filter(None, [
+            AddCompileFlag(lambda cfg: getSpeedOptimizationFlag(cfg)) if opt == "speed" else None,
+            AddCompileFlag(lambda cfg: getSizeOptimizationFlag(cfg)) if opt == "size" else None,
+        ]),
+    ),
     Parameter(
         name="enable_modules",
         choices=["none", "clang", "clang-lsv"],

@github-actions
Copy link

github-actions bot commented Oct 10, 2023

✅ With the latest revision this PR passed the Python code formatter.

@ldionne ldionne force-pushed the review/test-with-optimizations branch from 6d27185 to 9824ef1 Compare October 12, 2023 05:17
elif _isMSVC(cfg):
return "/O1"
else:
raise RuntimeError("Can't figure out what compiler is used in the configuration")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

black is unhappy with this formatting: #68753 (comment)

Personally, I'm not massively keen on the black coding style but it probably makes sense to keep it to something enforceable by a tool.

I do wonder if it would make sense to install it as a pre-commit hook to ensure the formatting stays consistent: https://github.com/akaihola/darker#using-as-a-pre-commit-hook

@ldionne ldionne requested a review from a team as a code owner November 6, 2023 00:31
Copy link

github-actions bot commented Nov 6, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 61968286f9a39815040b0d94299c3732834661bf 83e2758fc0c990a4d42b9573f513e1676a88504d -- libcxx/test/libcxx/depr/depr.default.allocator/allocator.members/allocate.cxx2a.pass.cpp libcxx/test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp libcxx/test/std/experimental/simd/simd.class/simd_ctor_conversion.pass.cpp libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/move.pass.cpp libcxx/test/std/input.output/filesystems/class.path/path.member/path.construct/move.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.indirect.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.replace.indirect.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.indirect.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.indirect.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size.replace.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.replace.indirect.pass.cpp libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.replace.indirect.pass.cpp libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.alg/swap.pass.cpp libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/F.pass.cpp libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_move.pass.cpp libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/nullptr_t_assign.pass.cpp libcxx/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.mod/swap.pass.cpp libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp libcxx/test/support/count_new.h libcxx/test/support/test_macros.h libunwind/test/libunwind_02.pass.cpp libunwind/test/unw_resume.pass.cpp libunwind/test/unwind_leaffunction.pass.cpp
View the diff from clang-format here.
diff --git a/libcxx/test/libcxx/depr/depr.default.allocator/allocator.members/allocate.cxx2a.pass.cpp b/libcxx/test/libcxx/depr/depr.default.allocator/allocator.members/allocate.cxx2a.pass.cpp
index f2fb606ee6..7ce5176d44 100644
--- a/libcxx/test/libcxx/depr/depr.default.allocator/allocator.members/allocate.cxx2a.pass.cpp
+++ b/libcxx/test/libcxx/depr/depr.default.allocator/allocator.members/allocate.cxx2a.pass.cpp
@@ -60,7 +60,7 @@ void test_aligned() {
   {
     globalMemCounter.last_new_size = 0;
     globalMemCounter.last_new_align = 0;
-    T* ap2 = a.allocate(11, (const void*)5);
+    T* ap2                          = a.allocate(11, (const void*)5);
     DoNotOptimize(ap2);
     assert(globalMemCounter.checkOutstandingNewEq(1));
     assert(globalMemCounter.checkNewCalledEq(1));
diff --git a/libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/move.pass.cpp b/libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/move.pass.cpp
index 93295d9f6d..7aa886ad31 100644
--- a/libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/move.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/move.pass.cpp
@@ -30,8 +30,7 @@ int main(int, char**) {
   assert(globalMemCounter.checkOutstandingNewEq(0));
   const std::string s("we really really really really really really really "
                       "really really long string so that we allocate");
-  ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS(
-      globalMemCounter.checkOutstandingNewLessThanOrEqual(1));
+  ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS(globalMemCounter.checkOutstandingNewLessThanOrEqual(1));
   const fs::path::string_type ps(s.begin(), s.end());
   path p(s);
   {
diff --git a/libcxx/test/std/input.output/filesystems/class.path/path.member/path.construct/move.pass.cpp b/libcxx/test/std/input.output/filesystems/class.path/path.member/path.construct/move.pass.cpp
index 3c762ee676..e51c9a8427 100644
--- a/libcxx/test/std/input.output/filesystems/class.path/path.member/path.construct/move.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/class.path/path.member/path.construct/move.pass.cpp
@@ -30,8 +30,7 @@ int main(int, char**) {
   assert(globalMemCounter.checkOutstandingNewEq(0));
   const std::string s("we really really really really really really really "
                       "really really long string so that we allocate");
-  ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS(
-      globalMemCounter.checkOutstandingNewLessThanOrEqual(1));
+  ASSERT_WITH_LIBRARY_INTERNAL_ALLOCATIONS(globalMemCounter.checkOutstandingNewLessThanOrEqual(1));
   const fs::path::string_type ps(s.begin(), s.end());
   path p(s);
   {
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.indirect.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.indirect.pass.cpp
index 8eaf50d538..31ed2488b4 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.indirect.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.indirect.pass.cpp
@@ -42,7 +42,7 @@ void operator delete(void* p) TEST_NOEXCEPT {
 
 int main(int, char**) {
     new_called = delete_called = 0;
-    int* x = DoNotOptimize(new int[3]);
+    int* x                     = DoNotOptimize(new int[3]);
     assert(x != nullptr);
     ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_called == 1);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.pass.cpp
index 9715bc9207..9fc96311d8 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size.replace.pass.cpp
@@ -40,7 +40,7 @@ void operator delete[](void* p) TEST_NOEXCEPT {
 
 int main(int, char**) {
     new_called = delete_called = 0;
-    int* x = DoNotOptimize(new int[3]);
+    int* x                     = DoNotOptimize(new int[3]);
     assert(x != nullptr);
     ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_called == 1);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.replace.indirect.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.replace.indirect.pass.cpp
index 66cbb4b9c8..fe79840671 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.replace.indirect.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align.replace.indirect.pass.cpp
@@ -51,7 +51,7 @@ int main(int, char**) {
     // Test with an overaligned type
     {
         new_called = delete_called = 0;
-        OverAligned* x = DoNotOptimize(new OverAligned[3]);
+        OverAligned* x             = DoNotOptimize(new OverAligned[3]);
         ASSERT_WITH_OPERATOR_NEW_FALLBACKS(static_cast<void*>(x) == DummyData);
         ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_called == 1);
 
@@ -62,7 +62,7 @@ int main(int, char**) {
     // Test with a type that is right on the verge of being overaligned
     {
         new_called = delete_called = 0;
-        MaxAligned* x = DoNotOptimize(new MaxAligned[3]);
+        MaxAligned* x              = DoNotOptimize(new MaxAligned[3]);
         assert(x != nullptr);
         assert(new_called == 0);
 
@@ -73,7 +73,7 @@ int main(int, char**) {
     // Test with a type that is clearly not overaligned
     {
         new_called = delete_called = 0;
-        int* x = DoNotOptimize(new int[3]);
+        int* x                     = DoNotOptimize(new int[3]);
         assert(x != nullptr);
         assert(new_called == 0);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.indirect.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.indirect.pass.cpp
index df8a651932..d3a795e9e9 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.indirect.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.indirect.pass.cpp
@@ -51,7 +51,7 @@ int main(int, char**) {
     // Test with an overaligned type
     {
         new_called = delete_called = 0;
-        OverAligned* x = DoNotOptimize(new (std::nothrow) OverAligned[3]);
+        OverAligned* x             = DoNotOptimize(new (std::nothrow) OverAligned[3]);
         ASSERT_WITH_OPERATOR_NEW_FALLBACKS(static_cast<void*>(x) == DummyData);
         ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_called == 1);
 
@@ -62,7 +62,7 @@ int main(int, char**) {
     // Test with a type that is right on the verge of being overaligned
     {
         new_called = delete_called = 0;
-        MaxAligned* x = DoNotOptimize(new (std::nothrow) MaxAligned[3]);
+        MaxAligned* x              = DoNotOptimize(new (std::nothrow) MaxAligned[3]);
         assert(x != nullptr);
         assert(new_called == 0);
 
@@ -73,7 +73,7 @@ int main(int, char**) {
     // Test with a type that is clearly not overaligned
     {
         new_called = delete_called = 0;
-        int* x = DoNotOptimize(new (std::nothrow) int[3]);
+        int* x                     = DoNotOptimize(new (std::nothrow) int[3]);
         assert(x != nullptr);
         assert(new_called == 0);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.pass.cpp
index b984e8cf0a..ade6fad7aa 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_align_nothrow.replace.pass.cpp
@@ -48,7 +48,7 @@ int main(int, char**) {
     // Test with an overaligned type
     {
         new_nothrow_called = delete_called = 0;
-        OverAligned* x = DoNotOptimize(new (std::nothrow) OverAligned[3]);
+        OverAligned* x                     = DoNotOptimize(new (std::nothrow) OverAligned[3]);
         assert(static_cast<void*>(x) == DummyData);
         ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_nothrow_called == 1);
 
@@ -59,7 +59,7 @@ int main(int, char**) {
     // Test with a type that is right on the verge of being overaligned
     {
         new_nothrow_called = delete_called = 0;
-        MaxAligned* x = DoNotOptimize(new (std::nothrow) MaxAligned[3]);
+        MaxAligned* x                      = DoNotOptimize(new (std::nothrow) MaxAligned[3]);
         assert(x != nullptr);
         assert(new_nothrow_called == 0);
 
@@ -70,7 +70,7 @@ int main(int, char**) {
     // Test with a type that is clearly not overaligned
     {
         new_nothrow_called = delete_called = 0;
-        int* x = DoNotOptimize(new (std::nothrow) int[3]);
+        int* x                             = DoNotOptimize(new (std::nothrow) int[3]);
         assert(x != nullptr);
         assert(new_nothrow_called == 0);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.indirect.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.indirect.pass.cpp
index 70d891b2a8..8cb853f828 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.indirect.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.indirect.pass.cpp
@@ -46,7 +46,7 @@ void operator delete(void* p) TEST_NOEXCEPT {
 
 int main(int, char**) {
     new_called = delete_called = 0;
-    int* x = DoNotOptimize(new (std::nothrow) int[3]);
+    int* x                     = DoNotOptimize(new (std::nothrow) int[3]);
     assert(x != nullptr);
     ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_called == 1);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.pass.cpp
index 2b8918276d..b390d9cfa0 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new.size_nothrow.replace.pass.cpp
@@ -35,7 +35,7 @@ void operator delete[](void* p) TEST_NOEXCEPT {
 
 int main(int, char**) {
     new_nothrow_called = delete_called = 0;
-    int* x = DoNotOptimize(new (std::nothrow) int[3]);
+    int* x                             = DoNotOptimize(new (std::nothrow) int[3]);
     assert(x != nullptr);
     ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_nothrow_called == 1);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size.replace.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size.replace.pass.cpp
index 3e0020b4f5..d4553a1208 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size.replace.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size.replace.pass.cpp
@@ -38,7 +38,7 @@ void operator delete(void* p) TEST_NOEXCEPT {
 
 int main(int, char**) {
     new_called = delete_called = 0;
-    int* x = DoNotOptimize(new int(3));
+    int* x                     = DoNotOptimize(new int(3));
     assert(x != nullptr);
     ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_called == 1);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.replace.indirect.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.replace.indirect.pass.cpp
index a68cdab545..e301b176f7 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.replace.indirect.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_align_nothrow.replace.indirect.pass.cpp
@@ -50,7 +50,7 @@ int main(int, char**) {
     // Test with an overaligned type
     {
         new_called = delete_called = 0;
-        OverAligned* x = DoNotOptimize(new (std::nothrow) OverAligned);
+        OverAligned* x             = DoNotOptimize(new (std::nothrow) OverAligned);
         ASSERT_WITH_OPERATOR_NEW_FALLBACKS(static_cast<void*>(x) == DummyData);
         ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_called == 1);
 
@@ -61,7 +61,7 @@ int main(int, char**) {
     // Test with a type that is right on the verge of being overaligned
     {
         new_called = delete_called = 0;
-        MaxAligned* x = DoNotOptimize(new (std::nothrow) MaxAligned);
+        MaxAligned* x              = DoNotOptimize(new (std::nothrow) MaxAligned);
         assert(x != nullptr);
         assert(new_called == 0);
 
@@ -72,7 +72,7 @@ int main(int, char**) {
     // Test with a type that is clearly not overaligned
     {
         new_called = delete_called = 0;
-        int* x = DoNotOptimize(new (std::nothrow) int);
+        int* x                     = DoNotOptimize(new (std::nothrow) int);
         assert(x != nullptr);
         assert(new_called == 0);
 
diff --git a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.replace.indirect.pass.cpp b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.replace.indirect.pass.cpp
index 64edbfd7e9..c9ef79a2ad 100644
--- a/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.replace.indirect.pass.cpp
+++ b/libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new.size_nothrow.replace.indirect.pass.cpp
@@ -41,7 +41,7 @@ void operator delete(void* p) TEST_NOEXCEPT {
 
 int main(int, char**) {
     new_called = delete_called = 0;
-    int* x = DoNotOptimize(new (std::nothrow) int(3));
+    int* x                     = DoNotOptimize(new (std::nothrow) int(3));
     assert(x != nullptr);
     ASSERT_WITH_OPERATOR_NEW_FALLBACKS(new_called == 1);
 
diff --git a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp
index be7505f218..d5120be8cf 100644
--- a/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp
+++ b/libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp
@@ -96,7 +96,7 @@ int main(int, char**)
     int i = 67;
     char c = 'e';
     std::shared_ptr<A> p = std::make_shared<A>(i, c);
-    assert(globalMemCounter.checkOutstandingNewLessThanOrEqual(nc+1));
+    assert(globalMemCounter.checkOutstandingNewLessThanOrEqual(nc + 1));
     assert(A::count == 1);
     assert(p->get_int() == 67);
     assert(p->get_char() == 'e');
@@ -116,7 +116,7 @@ int main(int, char**)
     {
     char c = 'e';
     std::shared_ptr<A> p = std::make_shared<A>(67, c);
-    assert(globalMemCounter.checkOutstandingNewLessThanOrEqual(nc+1));
+    assert(globalMemCounter.checkOutstandingNewLessThanOrEqual(nc + 1));
     assert(A::count == 1);
     assert(p->get_int() == 67);
     assert(p->get_char() == 'e');
diff --git a/libcxx/test/support/count_new.h b/libcxx/test/support/count_new.h
index ef4306e520..f6b2ad53e9 100644
--- a/libcxx/test/support/count_new.h
+++ b/libcxx/test/support/count_new.h
@@ -181,10 +181,7 @@ public:
         return disable_checking || n == outstanding_new;
     }
 
-    bool checkOutstandingNewLessThanOrEqual(int n) const
-    {
-        return disable_checking || outstanding_new <= n;
-    }
+    bool checkOutstandingNewLessThanOrEqual(int n) const { return disable_checking || outstanding_new <= n; }
 
     bool checkOutstandingNewNotEq(int n) const
     {
diff --git a/libcxx/test/support/test_macros.h b/libcxx/test/support/test_macros.h
index 5ca4d611e1..ac508ac2b7 100644
--- a/libcxx/test/support/test_macros.h
+++ b/libcxx/test/support/test_macros.h
@@ -291,13 +291,13 @@ struct is_same<T, T> { enum {value = 1}; };
 // when optimizations are enabled.
 template <class Tp>
 inline Tp const& DoNotOptimize(Tp const& value) {
-    asm volatile("" : : "r,m"(value) : "memory");
-    return value;
+  asm volatile("" : : "r,m"(value) : "memory");
+  return value;
 }
 
 template <class Tp>
 inline Tp& DoNotOptimize(Tp& value) {
-#if defined(__clang__)
+#  if defined(__clang__)
   asm volatile("" : "+r,m"(value) : : "memory");
 #else
   asm volatile("" : "+m,r"(value) : : "memory");

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I like the patch, some comments.

@ldionne ldionne force-pushed the review/test-with-optimizations branch from 8f9d777 to 2a5035e Compare November 17, 2023 21:39
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ldionne ldionne force-pushed the review/test-with-optimizations branch 2 times, most recently from 0cf1d0c to 1b132d6 Compare December 1, 2023 22:21
This patch adds a configuration of the libc++ test suite that enables
optimizations when building the tests. It also adds a new CI configuration
to exercise this on a regular basis. This is added in the context of [1],
which requires building with optimizations in order to hit the bug.

[1]: llvm#68552
@ldionne ldionne force-pushed the review/test-with-optimizations branch from 995337c to 66c9580 Compare January 8, 2024 21:07
@ldionne ldionne merged commit ca06c33 into llvm:main Jan 9, 2024
@ldionne ldionne deleted the review/test-with-optimizations branch January 9, 2024 15:39
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
This patch adds a configuration of the libc++ test suite that enables
optimizations when building the tests. It also adds a new CI
configuration to exercise this on a regular basis. This is added in the
context of [1], which requires building with optimizations in order to
hit the bug.

[1]: llvm#68552
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github:workflow libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libunwind test-suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants