From 5591f8a42997c7bbe99d26f7a75d494a53e436fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20H=C3=A4ssig?= Date: Mon, 24 Mar 2025 07:59:20 +0000 Subject: [PATCH] 8351515: C2 incorrectly removes double negation for double and float Reviewed-by: thartmann, chagedorn --- src/hotspot/share/opto/subnode.cpp | 13 ++- .../TestSubNodeFloatDoubleNegation.java | 80 +++++++++++++++++++ .../compiler/lib/ir_framework/IRNode.java | 2 +- 3 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/floatingpoint/TestSubNodeFloatDoubleNegation.java diff --git a/src/hotspot/share/opto/subnode.cpp b/src/hotspot/share/opto/subnode.cpp index de0d2f7c8d0..7ba6ac7a385 100644 --- a/src/hotspot/share/opto/subnode.cpp +++ b/src/hotspot/share/opto/subnode.cpp @@ -53,11 +53,16 @@ Node* SubNode::Identity(PhaseGVN* phase) { assert(in(1) != this, "Must already have called Value"); assert(in(2) != this, "Must already have called Value"); - // Remove double negation - const Type *zero = add_id(); - if( phase->type( in(1) )->higher_equal( zero ) && + const Type* zero = add_id(); + + // Remove double negation if it is not a floating point number since negation + // is not the same as subtraction for floating point numbers + // (cf. JLS § 15.15.4). `0-(0-(-0.0))` must be equal to positive 0.0 according to + // JLS § 15.8.2, but would result in -0.0 if this folding would be applied. + if (phase->type(in(1))->higher_equal(zero) && in(2)->Opcode() == Opcode() && - phase->type( in(2)->in(1) )->higher_equal( zero ) ) { + phase->type(in(2)->in(1))->higher_equal(zero) && + !phase->type(in(2)->in(2))->is_floatingpoint()) { return in(2)->in(2); } diff --git a/test/hotspot/jtreg/compiler/floatingpoint/TestSubNodeFloatDoubleNegation.java b/test/hotspot/jtreg/compiler/floatingpoint/TestSubNodeFloatDoubleNegation.java new file mode 100644 index 00000000000..66292e37b1c --- /dev/null +++ b/test/hotspot/jtreg/compiler/floatingpoint/TestSubNodeFloatDoubleNegation.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8351515 + * @summary The compiler must not fold `0-(0-x)` for x: float | double + * @library /test/lib / + * @modules jdk.incubator.vector + * @run driver compiler.floatingpoint.TestSubNodeFloatDoubleNegation + */ +package compiler.floatingpoint; + +import compiler.lib.ir_framework.*; +import jdk.incubator.vector.Float16; +import jdk.test.lib.Asserts; + +public class TestSubNodeFloatDoubleNegation { + + public static void main(String[] args) { + TestFramework.runWithFlags("--add-modules=jdk.incubator.vector", "-XX:CompileCommand=inline,jdk.incubator.vector.Float16::*"); + } + + @Run(test = { "testHalfFloat", "testFloat", "testDouble" }) + public static void assertResults() { + short halfFloatRes = Float16.float16ToShortBits(testHalfFloat(Float16.valueOf(-0.0f))); + int floatRes = Float.floatToIntBits(testFloat(-0.0f)); + long doubleRes = Double.doubleToLongBits(testDouble(-0.0)); + + Asserts.assertEQ((short) 0, halfFloatRes); + Asserts.assertEQ((int) 0, floatRes); + Asserts.assertEQ((long) 0, doubleRes); + } + + @Test + // Match a generic SubNode, because scalar Float16 operations are often + // performed as float operations. + @IR(counts = { IRNode.SUB, "2" }) + // Checks that the subtractions in 0 - (0 - hf) do not get eliminiated + public static Float16 testHalfFloat(Float16 hf) { + return Float16.subtract( + Float16.shortBitsToFloat16((short) 0), + Float16.subtract(Float16.shortBitsToFloat16((short) 0), hf)); + } + + @Test + @IR(counts = { IRNode.SUB_F, "2" }) + // Checks that the subtractions in 0 - (0 - f) do not get eliminated + public static float testFloat(float f) { + return 0 - (0 - f); + } + + @Test + @IR(counts = { IRNode.SUB_D, "2" }) + // Checks that the subtractions in 0 - (0 - d) do not get eliminated + public static double testDouble(double d) { + return 0 - (0 - d); + } + +} diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java index 6f66b403a09..25204dc280d 100644 --- a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java +++ b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java @@ -1833,7 +1833,7 @@ public class IRNode { public static final String SUB = PREFIX + "SUB" + POSTFIX; static { - beforeMatchingNameRegex(SUB, "Sub(I|L|F|D)"); + beforeMatchingNameRegex(SUB, "Sub(I|L|F|D|HF)"); } public static final String SUB_D = PREFIX + "SUB_D" + POSTFIX;