From ec277419b96c4d343a8374ec196d6c4b1f24d703 Mon Sep 17 00:00:00 2001 From: "E. G. Patrick Bos" Date: Fri, 14 Jun 2024 12:53:38 +0200 Subject: [PATCH 1/2] [RF] additional RooNaNPacker tests These are also sanity checks meant in part as code-as-documentation for users to see how things work. It turns out that GCC on the one hand and Clang and MSVC on the other hand have two different solutions for binary arithmetic operations on two NaNs. In both cases, one of the two NaNs is returned, but in one case it's the first, in the other is it's the second. The tests added in this commit will hopefully warn RooNaNPacker users of this behavior. --- roofit/roofitcore/test/testNaNPacker.cxx | 53 ++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/roofit/roofitcore/test/testNaNPacker.cxx b/roofit/roofitcore/test/testNaNPacker.cxx index c1f307ab60277..0d26663cafdb7 100644 --- a/roofit/roofitcore/test/testNaNPacker.cxx +++ b/roofit/roofitcore/test/testNaNPacker.cxx @@ -138,6 +138,59 @@ TEST(RooNaNPacker, PackedNaNPreservedAfterArithmetic) EXPECT_TRUE(rnp2.isNaNWithPayload()); // nothing can harm the PackedNaN EXPECT_EQ(rnp.getPayload(), rnp2.getPayload()); + + // multiply packed NaN by 0 + rnp2._payload = 0. * rnp.getNaNWithPayload(); + EXPECT_TRUE(rnp2.isNaNWithPayload()); + // PackedNaN pays no mind + EXPECT_EQ(rnp.getPayload(), rnp2.getPayload()); + + // the following tests have compiler dependent behavior + // (note that this behavior was tested for msvc as well, even though this + // whole test is currently disabled in msvc) +#if defined(__clang__) || defined(_MSC_VER) + // add packed NaN to regular NaN + rnp2._payload = std::numeric_limits::quiet_NaN() + rnp.getNaNWithPayload(); + // first NaN wins, though! now the payload is gone + EXPECT_FALSE(rnp2.isNaNWithPayload()); + // a quiet NaN has a "payload" of zero + EXPECT_EQ(0, rnp2.getPayload()); + + // ... other way around + rnp2._payload = rnp.getNaNWithPayload() + std::numeric_limits::quiet_NaN(); + EXPECT_TRUE(rnp2.isNaNWithPayload()); + // if it comes first, the PackedNaN does survive + EXPECT_EQ(rnp.getPayload(), rnp2.getPayload()); + + // multiply regular NaN with packed NaN + rnp2._payload = rnp.getNaNWithPayload() * std::numeric_limits::quiet_NaN(); + EXPECT_TRUE(rnp2.isNaNWithPayload()); + // if it comes first, the PackedNaN does survive + EXPECT_EQ(rnp.getPayload(), rnp2.getPayload()); + + // ... other way around + rnp2._payload = std::numeric_limits::quiet_NaN() * rnp.getNaNWithPayload(); + // same as with addition: the first NaN wins + EXPECT_FALSE(rnp2.isNaNWithPayload()); + // a quiet NaN has a "payload" of zero + EXPECT_EQ(0, rnp2.getPayload()); +#endif // defined(__clang__) || defined(_MSC_VER) +#if defined(__GNUC__) && !defined(__clang__) + // on gcc, a different NaN is preserved than in clang and msvc! + + // add packed NaN to regular NaN + rnp2._payload = rnp.getNaNWithPayload() + std::numeric_limits::quiet_NaN(); + // on gcc, the second NaN wins! now the payload is gone + EXPECT_FALSE(rnp2.isNaNWithPayload()); + // a quiet NaN has a "payload" of zero + EXPECT_EQ(0, rnp2.getPayload()); + + // ... other way around + rnp2._payload = std::numeric_limits::quiet_NaN() + rnp.getNaNWithPayload(); + EXPECT_TRUE(rnp2.isNaNWithPayload()); + // if it comes second, the PackedNaN does survive + EXPECT_EQ(rnp.getPayload(), rnp2.getPayload()); +#endif // __GNUC__ } #endif // !defined(_MSC_VER) || defined(R__ENABLE_BROKEN_WIN_TESTS) From 757457945512bc52b67ef15427f143b5ff470d67 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 10 Nov 2025 21:46:40 +0100 Subject: [PATCH 2/2] [RF] Remove compiler specific NaNPacker tests Nowadays, the behavior on all compilers seems to be the same. --- roofit/roofitcore/test/testNaNPacker.cxx | 25 ------------------------ 1 file changed, 25 deletions(-) diff --git a/roofit/roofitcore/test/testNaNPacker.cxx b/roofit/roofitcore/test/testNaNPacker.cxx index 0d26663cafdb7..f69bbdc627ab9 100644 --- a/roofit/roofitcore/test/testNaNPacker.cxx +++ b/roofit/roofitcore/test/testNaNPacker.cxx @@ -101,8 +101,6 @@ TEST(RooNaNPacker, CanPackStuffIntoNaNs) dumpFloats(rnp._payload); } -#if !defined(_MSC_VER) || defined(_WIN64) || defined(NDEBUG) || defined(R__ENABLE_BROKEN_WIN_TESTS) - // Demonstrate value preserving behavior after arithmetic on packed NaNs. TEST(RooNaNPacker, PackedNaNPreservedAfterArithmetic) { @@ -145,10 +143,6 @@ TEST(RooNaNPacker, PackedNaNPreservedAfterArithmetic) // PackedNaN pays no mind EXPECT_EQ(rnp.getPayload(), rnp2.getPayload()); - // the following tests have compiler dependent behavior - // (note that this behavior was tested for msvc as well, even though this - // whole test is currently disabled in msvc) -#if defined(__clang__) || defined(_MSC_VER) // add packed NaN to regular NaN rnp2._payload = std::numeric_limits::quiet_NaN() + rnp.getNaNWithPayload(); // first NaN wins, though! now the payload is gone @@ -174,27 +168,8 @@ TEST(RooNaNPacker, PackedNaNPreservedAfterArithmetic) EXPECT_FALSE(rnp2.isNaNWithPayload()); // a quiet NaN has a "payload" of zero EXPECT_EQ(0, rnp2.getPayload()); -#endif // defined(__clang__) || defined(_MSC_VER) -#if defined(__GNUC__) && !defined(__clang__) - // on gcc, a different NaN is preserved than in clang and msvc! - - // add packed NaN to regular NaN - rnp2._payload = rnp.getNaNWithPayload() + std::numeric_limits::quiet_NaN(); - // on gcc, the second NaN wins! now the payload is gone - EXPECT_FALSE(rnp2.isNaNWithPayload()); - // a quiet NaN has a "payload" of zero - EXPECT_EQ(0, rnp2.getPayload()); - - // ... other way around - rnp2._payload = std::numeric_limits::quiet_NaN() + rnp.getNaNWithPayload(); - EXPECT_TRUE(rnp2.isNaNWithPayload()); - // if it comes second, the PackedNaN does survive - EXPECT_EQ(rnp.getPayload(), rnp2.getPayload()); -#endif // __GNUC__ } -#endif // !defined(_MSC_VER) || defined(R__ENABLE_BROKEN_WIN_TESTS) - /// Fit a simple linear function, that starts in the negative. TEST(RooNaNPacker, FitSimpleLinear) {