RFR(M): Graal PTX enhancements

Gilles Duboscq duboscq at ssw.jku.at
Tue Apr 9 08:24:36 PDT 2013


It's not so important but there are still some System.err.println (in
PTXLIRGenerator.java, PTXCompare.java and PTXControlFlow.java)


On Tue, Apr 9, 2013 at 5:03 PM, Morris Meyer <morris.meyer at oracle.com>wrote:

>  Thanks Gilles, Doug and Christian for the review.  Here is my updated
> webrev addressing those concerns.
>
>         --mm
>
> WEBREV - http://cr.openjdk.java.net/~morris/Graal-PTX-Enhancements.03
>
>
> On 4/9/13 7:59 AM, Gilles Duboscq 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 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
>>>
>>>         --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> 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
>>>>>>
>>>>> 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