RFR(M): Graal PTX enhancements

Thomas Wuerthinger thomas.wuerthinger at oracle.com
Tue Apr 9 08:36:40 PDT 2013


Gilles,

Could you please add automatic sanity checks regarding System.err.println and System.out.println to our check style configuration (see http://checkstyle.sourceforge.net/config_regexp.html)?

Thanks, thomas

On Apr 9, 2013, at 5:24 PM, Gilles Duboscq <duboscq at ssw.jku.at> wrote:

> 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