webrev for remaining bitwise operators and arithmetic negation

[Venkatachalam, Vasanth]

Hi Doug,

Thanks for the feedback. I gather from below that you will proceed to integrate the patch making these changes along the way. If I need to do anything on my end, please let me know.

Please attribute the patch to Vasanth Venkatachalam.

Commit message: Adds support to the HSAIL backend for arithmetic negation and two of the bitwise logical operators, bitwise shift right (>>), and bitwise NOT (~) 

Vasanth


-----Original Message-----
From: Doug Simon [mailto:doug.simon at oracle.com] 
Sent: Friday, November 01, 2013 5:46 AM
To: Venkatachalam, Vasanth
Cc: graal-dev at openjdk.java.net; dl.Runtimes
Subject: Re: webrev for remaining bitwise operators and arithmetic negation

Somewhat related to Christian's comment, please don't use javadoc style comments where they are not applicable.

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

+    private void emitTextFormattedInstruction(String instr, Value dest, Value... sources) {
+        /**
+         * Destination can't be a constant and no instruction has > 3 source operands.
+         */
+        assert (!isConstant(dest) && sources.length <= 3);

Javadoc comments cannot be applied to statements.

Otherwise, the patch looks fine. I'll integrate it and fix up these comments along the way.

-Doug

On Oct 31, 2013, at 11:24 PM, Venkatachalam, Vasanth <Vasanth.Venkatachalam at amd.com> wrote:

> Hi,
> 
> I've uploaded a webrev that extends the HSAIL backend to generate code for bitwise right shift (>>), bitwise NOT (~), and arithmetic negation.
> 
> http://cr.openjdk.java.net/~tdeneau/webrev-bitwiseopsupport-parttwo.01/webrev/
> 
> Please review and provide feedback.
> 
> Summary of main changes.
> 
> 
> *         In HSAILLIRGenerator. Java I extended the implementation of the emitNegate, emitNot and emitShr routines to handle cases that weren't previously being covered. I then made supporting changes to HSAILArithmetic.java and HSAILAssembler.java.
> 
> *         I added unit test cases in compiler/hsail/test for left shift (<<), right shift (>>), unsigned right shift(>>>) and integer negation.
> 
> *         I refactored the emit routines in HSAILAssembler which were calling emitString. We previously had multiple of these routines taking different number of source operands. I was able to coalesce these into a single routine (renamed emitTextFormattedInstruction) taking variable number of source operands.  This makes the code more readable but doesn't impact functionality.
> 
> *         A stylistic change. I rearranged the opcode values in the HSAILArithmetic enum to appear in alphabetical order for easier readability.
> 
> *         I've run this code through the eclipse 4.3 formatter and didn't see any changes in files that are part of this webrev. I've also tested the webrev against the latest OpenJDK version of the trunk.
> 
> Vasanth