[compiler-rt] f668a84 - [scudo][standalone] Remove unused atomic_compare_exchange_weak
Kostya Kortchinsky via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 28 16:25:43 PDT 2020
Author: Kostya Kortchinsky
Date: 2020-09-28T16:25:14-07:00
New Revision: f668a84b58dcd816656236d7ba7e820dce52e7f8
URL: https://github.com/llvm/llvm-project/commit/f668a84b58dcd816656236d7ba7e820dce52e7f8
DIFF: https://github.com/llvm/llvm-project/commit/f668a84b58dcd816656236d7ba7e820dce52e7f8.diff
LOG: [scudo][standalone] Remove unused atomic_compare_exchange_weak
`atomic_compare_exchange_weak` is unused in Scudo, and its associated
test is actually wrong since the weak variant is allowed to fail
spuriously (thanks Roland).
This lead to flakes such as:
```
[ RUN ] ScudoAtomicTest.AtomicCompareExchangeTest
../../zircon/third_party/scudo/src/tests/atomic_test.cpp:98: Failure: Expected atomic_compare_exchange_weak(reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed) is true.
Expected: true
Which is: 01
Actual : atomic_compare_exchange_weak(reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed)
Which is: 00
../../zircon/third_party/scudo/src/tests/atomic_test.cpp:100: Failure: Expected atomic_compare_exchange_weak( reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed) is false.
Expected: false
Which is: 00
Actual : atomic_compare_exchange_weak( reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed)
Which is: 01
../../zircon/third_party/scudo/src/tests/atomic_test.cpp:101: Failure: Expected OldVal == NewVal.
Expected: NewVal
Which is: 24
Actual : OldVal
Which is: 42
[ FAILED ] ScudoAtomicTest.AtomicCompareExchangeTest (0 ms)
[----------] 2 tests from ScudoAtomicTest (1 ms total)
```
So I am removing this, if someone ever needs the weak variant, feel
free to add it back with a test that is not as terrible. This test was
initially ported from sanitizer_common, but their weak version calls
the strong version, so it works for them.
Differential Revision: https://reviews.llvm.org/D88443
Added:
Modified:
compiler-rt/lib/scudo/standalone/atomic_helpers.h
compiler-rt/lib/scudo/standalone/tests/atomic_test.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/scudo/standalone/atomic_helpers.h b/compiler-rt/lib/scudo/standalone/atomic_helpers.h
index 1ea1a86ae506..0946a3308172 100644
--- a/compiler-rt/lib/scudo/standalone/atomic_helpers.h
+++ b/compiler-rt/lib/scudo/standalone/atomic_helpers.h
@@ -106,14 +106,6 @@ inline bool atomic_compare_exchange_strong(volatile T *A, typename T::Type *Cmp,
__ATOMIC_RELAXED);
}
-template <typename T>
-inline bool atomic_compare_exchange_weak(volatile T *A, typename T::Type *Cmp,
- typename T::Type Xchg,
- memory_order MO) {
- return __atomic_compare_exchange(&A->ValDoNotUse, Cmp, &Xchg, true, MO,
- __ATOMIC_RELAXED);
-}
-
// Clutter-reducing helpers.
template <typename T>
diff --git a/compiler-rt/lib/scudo/standalone/tests/atomic_test.cpp b/compiler-rt/lib/scudo/standalone/tests/atomic_test.cpp
index 103cd24624ba..e90a642fd353 100644
--- a/compiler-rt/lib/scudo/standalone/tests/atomic_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/atomic_test.cpp
@@ -80,26 +80,14 @@ TEST(ScudoAtomicTest, AtomicStoreLoad) {
template <typename T> void checkAtomicCompareExchange() {
typedef typename T::Type Type;
- {
- Type OldVal = 42;
- Type NewVal = 24;
- Type V = OldVal;
- EXPECT_TRUE(atomic_compare_exchange_strong(
- reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed));
- EXPECT_FALSE(atomic_compare_exchange_strong(
- reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed));
- EXPECT_EQ(NewVal, OldVal);
- }
- {
- Type OldVal = 42;
- Type NewVal = 24;
- Type V = OldVal;
- EXPECT_TRUE(atomic_compare_exchange_weak(reinterpret_cast<T *>(&V), &OldVal,
+ Type OldVal = 42;
+ Type NewVal = 24;
+ Type V = OldVal;
+ EXPECT_TRUE(atomic_compare_exchange_strong(reinterpret_cast<T *>(&V), &OldVal,
NewVal, memory_order_relaxed));
- EXPECT_FALSE(atomic_compare_exchange_weak(
- reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed));
- EXPECT_EQ(NewVal, OldVal);
- }
+ EXPECT_FALSE(atomic_compare_exchange_strong(
+ reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed));
+ EXPECT_EQ(NewVal, OldVal);
}
TEST(ScudoAtomicTest, AtomicCompareExchangeTest) {
More information about the llvm-commits
mailing list