webrev to use oops_do to modify saved hsail state

[Deneau, Tom]

Gilles --

I put a new webrev up at
http://cr.openjdk.java.net/~tdeneau/graal-webrevs/webrev-hsail-oops-do-v2/webrev

-- Tom

-----Original Message-----
From: gilwooden at gmail.com [mailto:gilwooden at gmail.com] On Behalf Of Gilles Duboscq
Sent: Tuesday, June 24, 2014 8:18 AM
To: Deneau, Tom
Cc: graal-dev at openjdk.java.net
Subject: Re: webrev to use oops_do to modify saved hsail state

Hello Tom,

A few comments:
The indentation in ArrayListSetTest.java and
BoundsCatchMost20000StressGCTest.java is a bit strange.
In gpu_hsail.cpp: in execute_kernel_void_1d_internal we could asser
that THREAD is a JavaThread (is_Java_thread()).
In gpu_hsail_Frame.hpp: there starts to be a lot of hard coded
constants in there.
In gpu_hsail_OopMapHelper.hpp: _saveAreaCounts seems unused (it was
already unused in the OopSaver).

It looks a bit strange that the oop maps are java object. They should
probably be installed as native objects along the kernel but that's
outside the scope of this change.
As for this change, after the indentation changes in the java file and
removal of _saveAreaCounts I think it's fine.

-Gilles



On Mon, Jun 23, 2014 at 8:35 PM, Tom Deneau <tom.deneau at amd.com> wrote:
> Has anyone had a chance to look a this webrev?
>
> -- Tom
>
> From: Deneau, Tom
> Sent: Tuesday, June 17, 2014 10:21 AM
> To: graal-dev at openjdk.java.net
> Subject: webrev to use oops_do to modify saved hsail state
>
> I placed a webrev up at
> http://cr.openjdk.java.net/~tdeneau/graal-webrevs/webrev-hsail-oops-do/webrev/
> Please consider this for inclusion in graal.
>
> Background:  Ever since we started support deoptimization from hsail kernels, we have had to deal with the fact that any oops saved in the hsail deoptimization save area could move around by GC during the windows that open during deoptimizations to the interpreter.  Up until now, we had a hackier (and higher overhead) workaround involving copying oops to and from a java Object array and letting them get fixed up there if necessary.
>
> This webrev instead uses the standard process of updating the oops in the saved HSAIL state by calling oops_do from a GC thread.
>
> *         A field was added to JavaThread so that we could tell that a thread was processing an hsail kernel.    If so,  we go thru the saved hsail frames and apply the oops_do closure to each oop in each saved frame.
>
> *         Because the saved HSAILFrames are only used during deoptimization to the interpreter, we only need to process the frames for the deopt items that have not been handled yet.  A field was added to HSAILDeoptimizationInfo to keep track of this.
>
> *         Also, note that we don't need to do anything if we have finished the deopting workitems and are handling the never-ran workitems.  (The never-rans have to resolve the argument oops, but they always did that).
>
> There used to be a class called OopSaver defined in gpu_hsail.cpp.  That has been removed and we no longer pass the save array.  Some of the logic from OopSaver is now in OopMapHelper.  HSAILFrame was extended with a few new functions to help with oops_do.
>
> The disabling of ArrayListSetTest was because of an issue we saw unrelated to this webrev.  This issue will be fixed in a later webrev.
>
> Note: Running with the hsail simulator, there will be a small number of de-opts so it's not the best test environment for this feature, but tests have passed on HSA hardware boxes which can have thousands of de-opts.
>
> -- Tom
>
>