webrev: workaround for threadRegister handling

[Doug Simon]

Hi Vasanth,

I think you are tackling this problem at the wrong level. All the logic that uses threadRegister and stackPointerRegister is in snippets. The point of snippets is they are the interface the compiler uses to do runtime-specific lowering. I somehow very much doubt that the snippets using these registers will make any sense in the context of the GPU. Even if you plan on implementing new/newarray/monitorenter/monitorexit etc on the GPU, the code will be quite different than that for HotSpot's "host" runtime. After all, they are very specific to HotSpot data structures such as thread local allocation buffers, G1 barriers, etc.

In my opinion, you need to have a GraalCodeCacheProvider implementation that does all the GPU specific lowering. To ensure you've got this separation right/complete, your  
GraalCodeCacheProvider subclass probably shouldn't even subclass HotSpotRuntime.

-Doug

On Sep 10, 2013, at 8:23 PM, "Venkatachalam, Vasanth" <Vasanth.Venkatachalam at amd.com> wrote:

> Hi,
> 
> We've submitted the following webrev to http://cr.openjdk.java.net/~tdeneau/graal-webrevs/webrev-r15changes/webrev/
> 
> Please review it.
> 
> Details:
> 
> Graal contains some logic that calls the host runtime (x86 or Sparc) for information that is specific to the runtime that it is generating code for.
> An example is the method HotSpotRuntimeUtils.threadRegister(), which retrieves the thread register of the host runtime.
> This logic works when where the host runtime is the same as the runtime  that it is generating code for, but can break in cases where the two runtimes are different, such as when the JVM is running on x86 but generating code for HSAIL.
> 
> This webrev provides an initial workaround that we'd like to get feedback on. This isn't the final solution but is intended to set up for a better solution later, whether that comes from us or the community. 
> In detail, the webrev adds some routines to a central location (we chose HotSpotGraalRuntime). These routines can be used to set the target ISA runtime at startup or to query it. If the target ISA runtime isn't set, then it defaults to the host runtime. Currently we've commented out the code (in HSAILCompilationResult) which sets the target ISA runtime, so that the default behavior of using the host runtime is preserved. However, to help people see how this is meant to work, we've modified HotSpotReplacementUtils.threadRegister( ) to query the target ISA runtime for the thread register, which again defaults to the host runtime so that there's no change in behavior.
> 
> We're calling this a workaround because the problem of how to handle the thread register for HSAIL requires more investigation. In HSAIL, register values don't persist across function calls. Rather, every function gets its own "localized" view of the entire register set. So there's currently no way of fixing a single register as the "thread register" and having it persist across all calls. We would have to instead inject some code for each function call that sets up the thread register for that call. The details need to be worked out.
> 
> With this webrev (which includes some supporting changes to designate a "thread register for HSAIL), we were able to get past some Unit test failures where Graal was querying the host runtime for the thread register when we were compiling for HSAIL.
> 
> We'd welcome feedback and suggestions about alternative solutions.
> 
> Vasanth
> 
>