RFR(M): Graal PTX enhancements

Gilles Duboscq duboscq at ssw.jku.at
Tue Apr 9 04:59:48 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 Tue, Apr 9, 2013 at 1:59 PM, Gilles Duboscq <gilwooden at gmail.com> wrote:

> 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