webrev for switch statement support in HSAIL backend

[Venkatachalam, Vasanth]

Doug,

Answers inlined below.

If there are no other changes required please commit the webrev, citing Vasanth Venkatachalam as the author, with the commit message: "Adds support to the HSAIL backend for handling Java switch constructs .

Vasanth

-----Original Message-----
From: graal-dev-bounces at openjdk.java.net [mailto:graal-dev-bounces at openjdk.java.net] On Behalf Of Doug Simon
Sent: Tuesday, November 19, 2013 6:23 PM
To: graal-dev at openjdk.java.net
Subject: Re: webrev for switch statement support in HSAIL backend

graal/com.oracle.graal.asm.hsail/src/com/oracle/graal/asm/hsail/HSAILAssembler.java:

+            // Get a JNI reference handle to the object.
+            long refHandle = OkraUtil.getRefHandle(obj);

How does OkraUtil manage these handles? I hope it at least never creates more than one global JNI ref for any given object or has some way to release them when no longer needed.

>[Vasanth] OkraUtil never creates more than one JNI ref for any given object. 
This change isn't introducing anything new. This code has been in the assembler all along, we just extended it to remove the restriction that the Object passed as parameter to mov(Register reg, Object obj) must be an instance of Class.

graal/com.oracle.graal.lir.hsail/src/com/oracle/graal/lir/hsail/HSAILControlFlow.java:

+        // The array of key constants used for the cases of this switch statement.
+        @Use({CONST}) protected Constant[] keyConstants;
+        // The branch target labels that correspond to each case.
+        private final LabelRef[] keyTargets;
+        private LabelRef defaultTarget;
+        // The key of the switch. This will be compared with each of the keyConstants.
+        @Alive({REG}) protected Value key;
+
+        // Constructor. Called from the HSAILLIRGenerator.emitSequentialSwitch routine.
+        public SwitchOp(Constant[] keyConstants, LabelRef[] keyTargets, LabelRef defaultTarget, Value key) {

Java fields and methods should have javadoc comments instead of C++ style comments.

>>[Vasanth] This was my oversight. I've updated the latest version of the webrev to use Javadoc comments.

-Doug

On 11/20/2013 12:25 AM, Venkatachalam, Vasanth wrote:
> Hi,
>
> I've submitted a webrev for handling switch constructs generated by Java bytecode.
>
> Please review at http://cr.openjdk.java.net/~tdeneau/webrev-switchsupport.00/webrev/
>
> Notes:
>
> This webrev adds support for the IntegerSwitchNode. (Support for TypeSwitchNode may be added later.)
>
> The approach currently recommended for generating performant HSAIL code for Java switch statements is to generate a series of cascading compare and branch instructions (instead of doing a table switch).This is the approach that I have implemented here.
>
> I've added an additional enhancement to HSAILAssembler which is unrelated to the switch support.
> The routine mov(Register reg, Object obj) previously had a restriction that the Object parameter had to be an instance of Class. This restriction has been lifted.
>
> I ran this through the Eclipse 4.3 formatter and there weren't any file changes.
>
> Vasanth
>
>