graalCodeInstaller questions, recording scopes, etc.

[Doug Simon]

On Jan 2, 2014, at 10:40 PM, Deneau, Tom <tom.deneau at amd.com> wrote:

> See below...
> 
>> -----Original Message-----
>> From: Doug Simon [mailto:doug.simon at oracle.com]
>> Sent: Thursday, January 02, 2014 12:02 PM
>> To: Deneau, Tom
>> Cc: graal-dev at openjdk.java.net
>> Subject: Re: graalCodeInstaller questions, recording scopes, etc.
>> 
>> 
>> On Dec 20, 2013, at 3:41 PM, Deneau, Tom <tom.deneau at amd.com> wrote:
>> 
>>> Closing the loop again, for my original questions, adding answers
>>> below that were relayed in the Skype call on Thursday...
>>> 
>>>> -----Original Message-----
>>>> From: Deneau, Tom
>>>> Sent: Monday, December 16, 2013 3:58 PM
>>>> To: graal-dev at openjdk.java.net
>>>> Subject: graalCodeInstaller questions, recording scopes, etc.
>>>> 
>> 
>> <snip>
>> 
>>>   * Our infopoints have byteCodePositions.  The ones that get
>>>>     "recorded" go thru record_scope which has all sorts of
>>>>     host-specific checking on the scope info.  For instance,
>>>>     get_hotspot_value will compare a register # with
>>>>     RegisterImpl::number_of_registers (which refers to the host) and
>>>>     if it is, assert that the type is FLOAT or DOUBLE.  None of this
>>>>     holds with HSAIL registers.  What is the best way to resolve
>>>>     this?
>>>> 
>>> 
>>> This issue will be handled by the virtualization of some of the calls
>>> in graalCodeInstaller that Doug will be implementing.
>> 
>> I made the CodeInstaller class subclassable and the ScopeValue creation
>> methods virtual[1]. At some point, I imagine
>> CompilerToGPU.generateKernel() will be expanded to incorporate (and
>> obsolete) HotSpotCodeCacheProvider.addExternalMethod(). This means the
>> code for generateKernel in graalCompilerToGPU.cpp can use a GPU specific
>> subclass of CodeInstaller to create GPU ScopeValues.
>> 
>> -Doug
>> 
>> [1] http://hg.openjdk.java.net/graal/graal/rev/03bb0ee05409
> 
> Doug --
> 
> Just to make sure I understand how we should fit all this together...
> 
>  * HSAILCompilationResult currently calls providers.getCodeCache()).addExternalMethod
> 
>  * We already have a HSAILHotSpotCodeCacheProvider so we could override addExternalMethod with
>    our own version.
> 
>  * in our own version of addExternalMethod, we could call our own version of vm.installCodeHSAIL
>    (after adding that to the CompilerToVM interface, this seems messy.  Maybe there should be
>     a installCode in the CompilerToGPU interface)
> 
>  * installCodeHSAIL would have a line such as
>      HSAILCodeInstaller installer(compiled_code_handle, result, cb, installed_code_handle, triggered_deoptimizations_handle);
> 
>      where HSAILCodeInstaller extends CodeInstaller and uses the new virtual methods
> 
> 
> Or alternatively (and maybe this is what you are suggesting)
>  * HSAILCompilationResult calls toGPU.generateKernel before it calls providers.getCodeCache()).addExternalMethod
> 
>  * the logic for using HSAILCodeInstaller and the rest of the logic in graalCompilerToVM.installCode0 
>    could be added to gpu::hsail::generateKernel
> 
>  * we would still need some way to get the InstalledCode back (currently returned by addExternalMethod)
>    so we could store it in the HSAILCompilationResult

Yes, the latter is what I was suggesting. In CompilerToGPU, we’d replace:

    long generateKernel(byte[] code, String name) throws InvalidInstalledCodeException;

with:

    CodeInstallResult installKernel(HotSpotCompiledCode compiledCode, HotSpotInstalledCode code);

The implementation of this in CompilerToGPUImpl would closely resemble HotSpotCodeCacheProvider.addExternalMethod (which would be removed).

I’ve prototyped this change and have attached it as a patch (feel free to create and upload a webrev from it). You’d just need to make the changes in graalCompilerToGPU.cpp to implement CompilerToGPUImpl.installKernel0. Note that generateKernel is still a separate method so that a GPU kernel compilation can be performed without installing the result as an nmethod.

-Doug