webrev for Graal HSAIL backend

Sat Jun 15 13:20:15 PDT 2013

Hi Vasanth,

On Jun 14, 2013, at 10:34 PM, "Venkatachalam, Vasanth" <Vasanth.Venkatachalam at amd.com> wrote:

> Hi Doug,
> 
> Thanks for the feedback. We're addressing your comments and had a few questions.
> 
> 1)  I'm trying to understand the issue with the formatting. To apply the Eclipse formatter, I first ran mx ideinit, then started eclipse and under projects->properties->Java Code Style->Formatter, I made sure the profile (Unmanaged profile 'Graal') was selected. Is this the correct way to apply the formatter?

Yes. The Eclipse project configurations created by eclipseinit should also apply the formatter to any Java file when it is saved.

> Can you give an example of some code that wasn't formatted according to these rules?

Now that I've looked at the patch in more detail, I can't see any problem in terms of formatting.

> 2). The only projects that use JDK8 features are com.amd.sumatra, and com.oracle.graal.compiler.hsail.test.lambda, In mx/projects we have specified that these projects have Java compliance of 1.8, so they shouldn't be compiled if the JDK used is 1.7. It seems like there should be a way for Eclipse to pick up on these changes after running mx ideinit. Are you saying this isn't the case?

One problem is that Eclipse doesn't (yet) support JDK8 *language* features such as lambdas. Projects that only use JDK8 library features should be fine.

The ideinit command does not consider compliance level when generating Eclipse project configurations - it probably should. And it should probably ignore any projects whose compliance level is >= 8 altogether. Not only does Eclipse not yet support JDK8 anguage features, it cannot yet generate JDK8 class files (the maximum "Compiler compliance level" one can currently set in Eclipse is 1.7).

I've just modified the checkstyle command to pay attention to compliance levels. With this change, you should now be able to test your changes for Graal compliance on the command line with these commands:

mx clean
mx build --jdt /path/to/ecj.jar --jdt-warning-as-error
mx checkstyle

Once all 3 pass without any warnings or errors, the code should be easier to review and integrate. Note that these commands are part of our gate so we need to get them passing anyway. If you are game, you could try and replicate our complete gate testing with:

mx --jacoco on gate

BTW, if you download an ecj.jar[1] to <graal>/mx/ecj.jar, it will be automatically detected and used by the 'mx build' command, removing the need for the --jdt argument.

Let me know if you run into any problems or have any further questions.

-Doug

[1] http://www.eclipse.org/downloads/download.php?file=/eclipse/downloads/drops4/R-4.2.2-201302041200/ecj-4.2.2.jar

> -----Original Message-----
> From: graal-dev-bounces at openjdk.java.net [mailto:graal-dev-bounces at openjdk.java.net] On Behalf Of Doug Simon
> Sent: Thursday, June 13, 2013 9:04 AM
> To: Deneau, Tom
> Cc: sumatra-dev at openjdk.java.net; graal-dev at openjdk.java.net
> Subject: Re: webrev for Graal HSAIL backend
> 
> Tom,
> 
> It would really help to have this patch broken up into components that can be separately considered for integration. In particular (as Morris stated), having the HSAIL backend (and tests) in Graal would be great so a separate patch for this code would be a good first step.
> 
> One immediate observation is that a lot of the code does not yet import into Eclipse very well. This is the primary tool we use for development and code comprehension so getting the code into Eclipse compliant form would be very helpful in terms of being able to efficiently assess a contribution and offer further feedback. This means:
> 
> o Only use Java 7 language features (Eclipse does not yet support Java 8 language features and it's not clear when it will).
> o Remove all Checkstyle warnings (use the eclipse-cs plugin or 'mx checkstyle').
> o Remove all Java warnings (which show up in the Eclipse Problems view).
> o Format Java code with the Eclipse formatter (the Eclipse projects generated by 'mx eclipseinit' will ensure the Graal code formatting rules are used).
> 
> It would be useful to have a high level description of the code, broken down by package or collection of packages. This can either be plain text or (preferably) package-info.java files.
> 
> I notice there are files without licenses and some with University of Illinois licenses (the Okra framework). I'm no expert on licensing, but this may causes integration issues.
> 
> The HotSpot changes are probably best to keep in the Sumatra repository for now given their experimental nature.
> 
> I look forward to further progress in terms of getting Sumatra code into Graal - thanks for a good start!
> 
> -Doug
> 
> On Jun 12, 2013, at 5:59 PM, "Deneau, Tom" <tom.deneau at amd.com> wrote:
> 
>> Morris --
>> 
>> Regarding your comments on the hotspot changes in this webrev 
>> http://cr.openjdk.java.net/~ecaspole/graal_hsail/
>> I wanted to let you know that these hotspot changes really don't make 
>> sense on their own but co-operate with some JDK changes which we will 
>> be submitting in a separate webrev to the Sumatra-dev repository early 
>> next week (these JDK changes are much smaller than the graal changes).
>> 
>> Here is a brief overview of how the pieces fit together...
>> 
>> Basically we wanted the GPU offload programming model to be triggered 
>> by the programmer using Stream.parallel.forEach(). So the JDK changes 
>> are really just interceptions of the stream API for parallel forEach.
>> The intercept code tests whether the stream meets a few criteria to be 
>> offloadable, and if so tell graal (thru the Sumatra interface) to 
>> compile the lambda method to HSAIL and dispatch it.  If already 
>> compiled, it will just be dispatched.  Currently we have a property, 
>> off by default, which tells the JDK intercept code to offload 
>> immediately.  If the offload-immediate flag is set, no hotspot changes 
>> are really needed.
>> 
>> The hotspot changes just provide an alternate way of enabling the 
>> offloading (without using the offload-immediate flag) by using the 
>> compilation of the underlying lambda method as a trigger for 
>> offloading.  They are very experimental and we would welcome community 
>> input on other ways to do this.
>> 
>> Hope this helps...
>> 
>> -- Tom Deneau
>> 
>> 
>> 
>> -----Original Message-----
>> From: graal-dev-bounces at openjdk.java.net 
>> [mailto:graal-dev-bounces at openjdk.java.net] On Behalf Of Morris Meyer
>> Sent: Tuesday, June 11, 2013 7:17 PM
>> To: graal-dev at openjdk.java.net
>> Subject: Re: webrev for Graal HSAIL backend
>> 
>> Vasanth,
>> 
>> After seeing Apple's WWDC and the 4,000 core dual-GPU system they 
>> built into the Mac Pro, I'm very happy to see the work your team has 
>> put together.  Lots of good stuff here and I think we should take most of it.
>> 
>> I like that the HSAIL backend is in the com.oracle.graal namespace - 
>> not so much as an Oracle engineer - but it will make working and 
>> refactoring these GPU and CPU backends much easier.  Thanks.
>> 
>> compilerBroker.cpp, library_call.cpp, runtime.cpp and arguments.cpp seem 
>> like the might be a little specialized for this point in time.   Very 
>> interesting changes though.   I would like to get the heterogeneous 
>> method support into HotSpot / Graal and sit down at the language 
>> summit and discuss how we take on constructs like the ForEach work.
>> 
>> I think com.amd.sumatra.Sumatra is sort of in the right ballpark - it 
>> echos the change I made to CompilerToGPU in the earlier PTX work.  I 
>> would like of like to reserve the sumatra package for lambda work 
>> along the lines you are thinking for a collections / lambda oriented
>> java.lang.invoke set of code.   We need to get the requirements for this 
>> sort of externalized kernel creating defined as soon as we can.  Maybe 
>> ...bridge.gpu, ...bridge.gpu.hsail, ...bridge.gpu.ptx packages in the 
>> graal namespace?
>> 
>> I think it might be a good next step to put in the HSAIL back end and 
>> tests and the emulator working at the gate so we can build and verify
>> JDK9 / Sumatra / Graal changes in this environment going forward.
>> 
>> I will be working as time permits on the heterogeneous methods and PTX 
>> invocation so we can get both platforms at the gate integrating changes.
>> 
>> That's all for now.  I'm looking forward to working my way through all 
>> these unit tests.  Huge kudos AMD!
>> 
>>        --morris meyer
>> 
>> On 6/11/13 6:16 PM, Venkatachalam, Vasanth wrote:
>>> Hi,
>>> 
>>> The AMD Sumatra team has submitted a webrev (http://cr.openjdk.java.net/~ecaspole/graal_hsail/) that adds HSAIL code generation support for Graal, allowing Java programs to be compiled and executed on HSAIL-enabled GPU/APU devices. While this work is a prototype, we have included several working unit test cases, including Mandelbrot and NBody.
>>> 
>>> Features
>>> 
>>> Arithmetic operations for integers, longs, doubles, and floats Loads, 
>>> stores and move operations Min/max/rem/carry operations for integers 
>>> and longs Conversion operations - (currently support conversions 
>>> between integers and floats, integers and doubles, integers and longs, floats and doubles).
>>> Some math library operations (e.g., square root).
>>> Support for JDK8 lambda constructs.
>>> 
>>> Known Issues
>>> 
>>> -The logic to handle register spilling is work-in-progress, so not all test cases that induce spilling are guaranteed to work.
>>> -X86 register encodings are being passed to the HSAIL backend. The 
>>> calling convention returned by getCallingConvention() currently returns an x86 calling convention -Function call support has yet to be implemented.
>>> 
>>> For a detailed list of unsupported features, refer to the routines 
>>> that are emitting "NYI" in HSAILLIRGenerator.java
>>> 
>>> The test cases (except for BasicHSAILTest) require an HSAIL simulator or hardware to execute, but in lieu of a simulator or hardware they will output the HSAIL code generated, which is useful for debugging. Moreover, BasicHSAILTest provides a template for adding Java code snippets and viewing the HSAIL generated code without executing the code.
>>> 
>>> We encourage the community to support this new backend and extend it with additional features.
>>> 
>>> Vasanth
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>> 
>> 
>> 
> 
> 
> 