[9] RFR(S): 8074869: C2 code generator can replace -0.0f with +0.0f on Linux (original) (raw)

Zoltán Majó zoltan.majo at oracle.com
Fri Mar 13 18:29:11 UTC 2015

Thank you, Vladimir, Volker, and Dean, for the review! (Dean gave his feedback in a private discussion.)

Here is the patch I intend to push:

http://cr.openjdk.java.net/~zmajo/8074869/webrev.01/

The patch includes Volker's test.

Thank you and best regards,

Zoltan

On 03/13/2015 04:54 PM, Volker Simonis wrote:

Hi Zoltan,

I've tested the change on Linux/ppc64 and AIX and it works fine. However I don't really see the bug which you pretend to fix with this change. All the Linux architectures which are in the OpenJDK use the following predicate to check for +0.0: predicate((n->getf() == 0) && (fpclassify(n->getf()) == FPZERO) && (signbit(n->getf()) == 0)); In particular they explicitly check the sign bit of the float/double argument which should ensure correct operation. I wrote a small regression test (added to the new webrev for your convenience [1]) which proves the correct operation with the current code. If I remove the "signbit" check the regression test will fail. Nevertheless I think your change is good because it is a nice cleanup and simplification. So please go ahead and push it. Regards, Volker PS: I've also removed some trailing whitespace from your original change [1] http://cr.openjdk.java.net/~simonis/webrevs/2015/8074869/

On Thu, Mar 12, 2015 at 1:43 PM, Zoltán Majó <zoltan.majo at oracle.com> wrote: Hi,

please review the following small patch. Bug: https://bugs.openjdk.java.net/browse/JDK-8074869 Problem: On Linux, the C2 code generator can replace the value -0.0f with +0.0f (and also the value -0.0d with +0.0d). The reason is that in some *.ad files both the value -0.0f and +0.0f is treated as being +0.0f and can therefore be replaced with an immediate +0.0f embedded into an instruction. For example, in the sparc.ad file, the 'fpclass' function is used to decide if a float node's content is +0.0: predicate((n->getf() == 0) && (fpclass(n->getf()) == FPPZERO)); On Solaris, 'fpclass' returns FPPZERO if the parameter is +0.0f and FPNZERO if the parameter is -0.0f. As a result, +0.0f and -0.0f are distinguished by the compiler. On Linux, however, 'fpclass' is not available and therefore 'fpclassify' is used. 'fpclassify' does not distinguish between ±0.0f, it returns FPZERO for both +0.0f and -0.0f. Solution: Instead of 'fpclass', use cast float->int and double->long to check if value is +0.0f and +0.0d, respectively. This logic is already use on some architectures, for example on x8632 and on x8664. As 'fpclass' is not used any more, remove its declarations from globalDefintions*. The include of ieeefp.h must be kept as we rely on some other functionality from this header on solaris. Webrev: http://cr.openjdk.java.net/~zmajo/8074869/webrev.00/ Testing: - JPRT testing on all supported platforms (does not include aarch64 and ppc64) - manual testing on aarch64: All DaCapo benchmarks with the small input size. I used the default JVM flags and tested the VM w/ and w/o the fix. All benchmarks pass except eclipse. For eclipse, the same Java-level failure appears both w/ and w/o the fix, so I think the problem with eclipse is not due to these changes. I also tested with the "-Xcomp -XX:-TieredCompilation -server" flags. Eclipse fails in this case as well. Additionally, tradebeans and tradesoap fail with a Java-level failure. As the failure happens also with both builds (w/ and w/o the fix), I don't think the problem is caused by these changes either. - no testing on ppc64: I don't have access to a ppc64 machine. Could somebody with access to a ppc64 machine please build and test the VM with this patch (and then maybe confirm that it works)? Thank you and best regards, Zoltan

More information about the hotspot-compiler-dev mailing list