logic in Math.nextAfter for handling -0.0 input

[Deneau, Tom]

OK, I took the LongSubstitutions.java which were already part of the AMD64 backend.
I made the following (garbage) change just to see what code got generated...

    @MethodSubstitution
    public static int numberOfTrailingZeros(long i) {
        if (i == 0) {
            return 67;
        }
        return 13 + Long.numberOfTrailingZeros(i);
        // return BitScanForwardNode.scan(i);
    }


Then I tried the junit test shown below to get it to compile...
I ran this test using
mx --vm server --vmbuild product unittest BasicTest3

and got similar output that I got with the HSAIL backend.
(output shown below the source code...)

-- Tom

==========================================================
package com.oracle.graal.compiler.test;

import org.junit.*;

import com.oracle.graal.api.code.*;
import com.oracle.graal.debug.*;
import com.oracle.graal.nodes.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;

public class BasicTest3 extends GraalCompilerTest {
    @Test
    public void testitNTZ() {
        test("numTrailingZeros7");
    }

    public int numTrailingZeros7(long x) {
        return Long.numberOfTrailingZeros(x) + 7;
    }

    private void test(String snippet) {
        StructuredGraph graph = parse(snippet);
        InstalledCode code = getCode(graph.method(), graph);
        Debug.dump(graph, "Graph");

        // DisassemblerProvider dis = Graal.getRuntime().getCapability(DisassemblerProvider.class);
        DisassemblerProvider dis = getBackend().getDisassembler();
        if (dis != null) {
            String disasm = dis.disassemble(code);
            TTY.println("code generated for " + snippet + "\n" + disasm);
        } else {
            System.out.println("disassembler is null");
        }
    }
}
==========================================================

java.lang.RuntimeException: Exception while intercepting exception
	at com.oracle.graal.debug.internal.DebugScope.interceptException(DebugScope.java:314)
	at com.oracle.graal.debug.internal.DebugScope.handle(DebugScope.java:265)
	at com.oracle.graal.debug.Debug.handle(Debug.java:343)
	at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.makeGraph(ReplacementsImpl.java:475)
	at com.oracle.graal.replacements.ReplacementsImpl.makeGraph(ReplacementsImpl.java:410)
	at com.oracle.graal.replacements.ReplacementsImpl.getMethodSubstitution(ReplacementsImpl.java:281)
	at com.oracle.graal.phases.common.inlining.InliningUtil.getIntrinsicGraph(InliningUtil.java:500)
	at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.buildGraph(ReplacementsImpl.java:612)
	at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.parseGraph(ReplacementsImpl.java:514)
	at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.makeGraph(ReplacementsImpl.java:464)
	at com.oracle.graal.replacements.ReplacementsImpl.makeGraph(ReplacementsImpl.java:410)
	at com.oracle.graal.replacements.ReplacementsImpl.getMethodSubstitution(ReplacementsImpl.java:281)
	at com.oracle.graal.phases.common.inlining.InliningUtil.getIntrinsicGraph(InliningUtil.java:500)
	at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.buildGraph(ReplacementsImpl.java:612)
	at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.parseGraph(ReplacementsImpl.java:514)
	at com.oracle.graal.replacements.ReplacementsImpl$GraphMaker.makeGraph(ReplacementsImpl.java:464)
	at com.oracle.graal.replacements.ReplacementsImpl.makeGraph(ReplacementsImpl.java:410)
	at com.oracle.graal.replacements.ReplacementsImpl.getMethodSubstitution(ReplacementsImpl.java:281)
	[ ... repeating many times ...]




-----Original Message-----
From: Tom Rodriguez [mailto:tom.rodriguez at oracle.com] 
Sent: Friday, June 27, 2014 7:13 PM
To: Deneau, Tom
Cc: graal-dev at openjdk.java.net
Subject: Re: logic in Math.nextAfter for handling -0.0 input

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> 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
> 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> 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'
>> 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'
>> 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
>