webrev for remaining bitwise operators and arithmetic negation

[Christian Thalinger]

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

I don’t think you need the comments explaining what instruction you emit, like:

             case LSHL:
+                /**
+                 * Emit the HSAIL instruction for a left shift.
+                 */
                 masm.emit("shl", dst, src1, src2);

It’s pretty clear from the enum value.

Otherwise this looks good.

On Oct 31, 2013, at 3: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