webrev for remaining bitwise operators and arithmetic negation
Doug Simon
doug.simon at oracle.com
Fri Nov 1 03:46:23 PDT 2013
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
More information about the graal-dev
mailing list