RFR: 8319705: RISC-V: signumF/D intrinsics fails compiler/intrinsics/math/TestSignumIntrinsic.java [v2]

[Fei Yang]

On Wed, 8 Nov 2023 12:36:16 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you review the patch to fix the bug introduced by [JDK-8317971](https://bugs.openjdk.org/browse/JDK-8317971) which implemented signumF/D intrinsics.
>> But seems the intrinsics failed since day 1 (I checkouted the commit to verify it.)
>> Thanks
>> 
>> ## Test
>> hotspot tests:
>> 
>> test/hotspot/jtreg/compiler/intrinsics/math/TestSignumIntrinsic.java
>> test/hotspot/jtreg/compiler/c2/PolynomialRoot.java
>> 
>> 
>> other jdk tests contains Math.signum(...):
>> 
>> test/jdk/java/math/BigInteger/BigIntegerTest.java 
>> test/jdk/java/math/BigInteger/largeMemory/DivisionOverflow.java
>> test/jdk/java/math/BigInteger/SerializationTests.java
>> test/jdk/java/math/BigInteger/ModInvTime.java
>> test/jdk/java/math/BigDecimal/SquareRootTests.java
>> test/jdk/java/math/BigDecimal/DivideTests.java
>> test/jdk/java/lang/Math/IeeeRecommendedTests.java
>> test/jdk/java/lang/Math/IeeeRemainderTests.java
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove rFlagsReg

OK. I can now reproduce the bug with following command on sifive unmatched board:

make test TEST="./test/hotspot/jtreg/compiler/intrinsics/math/TestSignumIntrinsic.java" JTREG="VM_OPTIONS=-XX:-TieredCompilation"

Error message:

java.lang.RuntimeException: Unexpected double result from 123.4: expected 1.0 to equal 123.4
        at jdk.test.lib.Asserts.fail(Asserts.java:634)
        at jdk.test.lib.Asserts.assertEquals(Asserts.java:205)
        at jdk.test.lib.Asserts.assertEQ(Asserts.java:178)
        at compiler.intrinsics.math.TestSignumIntrinsic.doubleTest(TestSignumIntrinsic.java:114)
        at compiler.intrinsics.math.TestSignumIntrinsic.main(TestSignumIntrinsic.java:91)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
        at java.base/java.lang.Thread.run(Thread.java:1570)

JavaTest Message: Test threw exception: java.lang.RuntimeException: Unexpected double result from 123.4: expected 1.0 to equal 123.4


But it does not always reproduce as I think it depends on C2 register allocation.
For `C2_MacroAssembler::signum_fp`, it has three FP registers: `dst`, `src`, and `one`. And it is possible (and I have verified) that `dst` and `one` could be assigned the same FP register. This will be a problem after change [1]. This change copies `src` to `dst` on entry, thus modifies `one` in that case. This is erroneous as `one` is still alive here and will be used later by `fsgnj_d/s` if `src` is not NaN or +/-0.0.

[1] https://github.com/openjdk/jdk/pull/16186/commits/867d6e8ea2a53ae71fb8fd341892e8f88347ec07

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16557#issuecomment-1803035865