RFR(M): Graal PTX enhancements
Gilles Duboscq
gilwooden at gmail.com
Tue Apr 9 04:59:29 PDT 2013
There are some System.(out|err).println in various files could you remove
them? When there are things like
throw GraalInternalError.shouldNotReachHere(), if you need some output you
can also use GraalInternalError.shouldNotReachHere(String message) instead
of the println
Otherwise it looks good
On Sat, Apr 6, 2013 at 1:18 AM, Morris Meyer <morris.meyer at oracle.com>wrote:
> Folks
>
> There is an updated webrev with Christian's review changes here:
>
> WEBREV - http://cr.openjdk.java.net/~**morris/Graal-PTX-Enhancements.**02<http://cr.openjdk.java.net/~morris/Graal-PTX-Enhancements.02>
>
> --morris
>
>
> On 4/5/13 6:09 PM, Morris Meyer wrote:
>
>> Thanks for the review.
>>
>> Actually with JavaFX, explicit class declarations were part of our
>> check-in procedures. I put the changes into PTXLIRGenerator as IMHO it
>> helps folks just looking at the source to see what makes up the class.
>>
>> Will change the class to PTXTestBase and look after the indenting.
>>
>> --mm
>>
>>
>> On Apr 5, 2013, at 5:42 PM, Christian Thalinger <
>> christian.thalinger at oracle.**com <christian.thalinger at oracle.com>> wrote:
>>
>> On Apr 5, 2013, at 10:46 AM, Morris Meyer <morris.meyer at oracle.com>
>>> wrote:
>>>
>>> Folks,
>>>>
>>>> Could I get a review of my recent PTX enhancements to Graal? I've
>>>> changed the tests to have a base class, extended the assembler, and have
>>>> taken the test coverage of PTX from 2 to 23 tests.
>>>>
>>>> Thanks in advance,
>>>>
>>>> --morris meyer
>>>>
>>>> WEBREV - http://cr.openjdk.java.net/~**morris/Graal-PTX-Enhancements.**
>>>> 01 <http://cr.openjdk.java.net/~morris/Graal-PTX-Enhancements.01>
>>>>
>>> This is great!
>>>
>>> Looking at:
>>>
>>> graal/com.oracle.graal.**compiler.ptx/src/com/oracle/**
>>> graal/compiler/ptx/**PTXLIRGenerator.java
>>>
>>> it seems we need to turn off "Organize imports" in our favorite editor.
>>> Also indenting seems to be an issue.
>>>
>>> I know the answer is "use the mx script to create the IDE configuration"
>>> but no everybody is using Eclipse or Netbeans. We need to find a way to
>>> keep this consistent without the help of an IDE (maybe Mercurial push
>>> hooks?).
>>>
>>> graal/com.oracle.graal.**compiler.ptx.test/src/com/**
>>> oracle/graal/compiler/ptx/**test/PTXCompilerBase.java:
>>>
>>> Can we name the test base class PTXTestBase?
>>>
>>> Otherwise this looks good to me.
>>>
>>> -- Chris
>>>
>>
>
More information about the graal-dev
mailing list