RFR(M): Graal PTX enhancements

Morris Meyer morris.meyer at oracle.com
Tue Apr 9 08:03:43 PDT 2013


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 
> <mailto: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 <mailto: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/%7Emorris/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
>             <mailto:christian.thalinger at oracle.com>> wrote:
>
>                 On Apr 5, 2013, at 10:46 AM, Morris Meyer
>                 <morris.meyer at oracle.com
>                 <mailto: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/%7Emorris/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