logic in Math.nextAfter for handling -0.0 input
Tom Rodriguez
tom.rodriguez at oracle.com
Mon Jun 30 18:23:07 UTC 2014
Yes, this is something I’ll be fixing as well. We’re being very conservative about what we allow to be deleted from snippets but when preparing method substitutions we can end up inlining asserts which will get trimmed away and that’s ok. I’ll be fixing that. You can disable that assert until I push my fix.
tom
On Jun 30, 2014, at 11:20 AM, Deneau, Tom <tom.deneau at amd.com> wrote:
> This calling back to the original method as Tom Rodriguez suggested seemed to work fine for some substitutions, but then I hit this one which I don't really understand.
>
> @MethodSubstitution
> public static double nextAfter(double x, double direction) {
> double input = (x == -0.0 ? 0.0 : x);
> return nextAfter(input, direction);
> }
>
>
> java.lang.AssertionError: deleted side effecting node
> at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.finalizeGraph(ReplacementsImpl.java:489)
> at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.makeGraph(ReplacementsImpl.java:468)
> at com.oracle.graal.replacements.ReplacementsImpl.makeGraph(ReplacementsImpl.java:409)
> at com.oracle.graal.replacements.ReplacementsImpl.getMethodSubstitution(ReplacementsImpl.java:281)
> at com.oracle.graal.hotspot.hsail.HSAILHotSpotReplacementsImpl.getMethodSubstitution(HSAILHotSpotReplacementsImpl.java:132)
> at com.oracle.graal.phases.common.inlining.InliningUtil.getIntrinsicGraph(InliningUtil.java:500)
> at com.oracle.graal.phases.common.inlining.info.elem.InlineableGraph.getOriginalGraph(InlineableGraph.java:72)
> at com.oracle.graal.phases.common.inlining.info.elem.InlineableGraph.<init>(InlineableGraph.java:60)
> at com.oracle.graal.phases.common.inlining.info.elem.Inlineable.getInlineableElement(Inlineable.java:41)
>
> -----Original Message-----
> From: graal-dev [mailto:graal-dev-bounces at openjdk.java.net] On Behalf Of Deneau, Tom
> Sent: Monday, June 30, 2014 12:43 PM
> To: Tom Rodriguez
> Cc: graal-dev at openjdk.java.net
> Subject: RE: logic in Math.nextAfter for handling -0.0 input
>
> Thanks, Tom, I will give this a try...
>
>
>
> From: Tom Rodriguez [mailto:tom.rodriguez at oracle.com]
> Sent: Monday, June 30, 2014 12:17 PM
> To: Deneau, Tom
> Cc: graal-dev at openjdk.java.net
> Subject: Re: logic in Math.nextAfter for handling -0.0 input
>
> The meaning of "just call it" is the problem here. Currently the logic in ReplacementsImpl.java around line 595 is looking for a call to the substitution method and replacing that with an inline of the original method. It's not looking for invokes of the original method. So if you write your replacement like this:
>
>
> @MethodSubstitution
> public static double nextAfter(double x, double direction) {
> // work around special case of -0.0
> double xx = (x == -0.0 ? 0.0 : x);
> return Math.nextAfter(xx, direction);
> }
>
> it will stack overflow like you are seeing because it's recursively trying to do the method substitution. If you remove the "Math." part then it will look like a recursive call to the substitution method and be handled by replacing the call with the original Math.nextAfter code. I'll fix the code to properly handle calling either the original method or the substitution method so this works as expected.
>
> tom
>
> On Jun 27, 2014, at 5:13 PM, Tom Rodriguez <tom.rodriguez at oracle.com<mailto:tom.rodriguez at oracle.com>> wrote:
>
>
> That's supposed to work correctly. I suppose it could be because of inline everything though I'm not sure how that would trigger the problem. Can try running your problematic test case against regular graal and see if it reproduces?
>
> I've seen cases like this with recursive deopts. Maybe that's what's happening here?
>
> tom
>
> On Jun 27, 2014, at 4:40 PM, Deneau, Tom <tom.deneau at amd.com<mailto:tom.deneau at amd.com>> wrote:
>
>
> That's what I was doing below. Is the problem that we are "inlining everything" on the hsail backend?
>
> -- Tom
>
> -----Original Message-----
> From: Tom Rodriguez [mailto:tom.rodriguez at oracle.com]
> Sent: Friday, June 27, 2014 6:39 PM
> To: Deneau, Tom
> Cc: graal-dev at openjdk.java.net<mailto:graal-dev at openjdk.java.net>
> Subject: Re: logic in Math.nextAfter for handling -0.0 input
>
> Just call it. A call to the original method stays a real call to that method and doesn't get substituted.
>
> tom
>
> On Jun 27, 2014, at 4:35 PM, Deneau, Tom <tom.deneau at amd.com<mailto:tom.deneau at amd.com>> wrote:
>
>
> Pinging, since I never saw an answer to this.
>
> I thought there was some way to substitute for a method and still call
> the original method without getting into a recursive call...
>
> (The original problem below is resolved but need this kind of solution for a different reason).
>
> -- Tom
>
>
> -----Original Message-----
> From: Deneau, Tom
> Sent: Monday, June 23, 2014 3:43 PM
> To: Deneau, Tom; 'graal-dev at openjdk.java.net<mailto:graal-dev at openjdk.java.net>'
> Subject: RE: logic in Math.nextAfter for handling -0.0 input
>
> Related to this...
> Should I be able to work around this with the following hsail MethodSubstitution?
> When I try this, I get a StackOverflowError...
>
> -- Tom
>
> @ClassSubstitution(java.lang.Math.class)
> public class HSAILMathSubstitutions {
> ...
> @MethodSubstitution
> public static double nextAfter(double x, double direction) {
> // work around special case of -0.0
> double xx = (x == -0.0 ? 0.0 : x);
> return Math.nextAfter(xx, direction);
> }
> }
>
>
>
>
> -----Original Message-----
> From: Deneau, Tom
> Sent: Monday, June 23, 2014 3:34 PM
> To: 'graal-dev at openjdk.java.net<mailto:graal-dev at openjdk.java.net>'
> Subject: logic in Math.nextAfter for handling -0.0 input
>
> The JDK method Math.nextAfter contains the logic shown below to handle
> an input of -0.0
>
> When the graal compiler compiles this for the hsail backend, it makes the reasonable assumption that "start + 0.0d" can be reduced to "start" which is a problem for this algorithm.
>
> From what I could tell, c2 or graal for amd64 backend do not do this optimization and so get the right answer for -0.0 input. How do they know not to do this optimization?
>
>
> } else { // start > direction or start < direction
> // Add +0.0 to get rid of a -0.0 (+0.0 + -0.0 => +0.0)
> // then bitwise convert start to integer.
> long transducer = Double.doubleToRawLongBits(start + 0.0d); <==============
>
> if (direction > start) { // Calculate next greater value
> transducer = transducer + (transducer >= 0L ? 1L:-1L);
> } else { // Calculate next lesser value
> ....
>
> -- Tom
More information about the graal-dev
mailing list