graalCodeInstaller questions, recording scopes, etc.

[Deneau, Tom]

Doug --

Actually you did send this on Jan 2.  And thanks, it was a good start to getting things coded up here.

Our status as of now is:

*         have an c2v_installKernel0 (borrowed heavily from installCode0)

*         This calls a gpu function to get the backend-specific CodeInstaller derived class, then calls the install method using that installer.

*         For HSAIL backend, it returns an HsailCodeInstaller.  Where we have this dummy get_scope_value for now which lets us at least finish the code install.
ScopeValue* HsailCodeInstaller::get_scope_value(oop value, int total_frame_size, GrowableArray<ScopeValue*>* objects, ScopeValue* &second, OopRecorder* oop_recorder) {
    second = NULL;
    return new LocationValue(Location::new_stk_loc(Location::invalid, 0));
}

Then at runtime we can find the scope based on the PC using
    ScopeDesc *scope = nm->scope_desc_at(pc);

And from that get the exception method and bci.   Because get_scope_value is just a skeleton, we can't yet at runtime deduce the registers or build any kind of vframe.  However, as you mentioned on the call, if the bci is 0, we can use a javaCalls call to run the kernel (we know what the kernel arguments are).

By the way, the stack corruption thing I was hunting for yesterday was caused by some not well thought out code that returned the CodeInstaller object by value.  I have reverted to returning it as a pointer and deriving CodeInstaller from CHeapObj<mtInternal>.  I don't know if that's the right way to do this, let me know if it is not.

-- Tom



From: Doug Simon [mailto:doug.simon at oracle.com]
Sent: Tuesday, January 07, 2014 4:09 PM
To: Deneau, Tom; S. Bharadwaj Yadavalli
Subject: Re: graalCodeInstaller questions, recording scopes, etc.

I don't think I actually attached the patch mentioned below - here it is.

On Jan 3, 2014, at 12:17 AM, Doug Simon <doug.simon at oracle.com<mailto:doug.simon at oracle.com>> wrote:

>
> On Jan 2, 2014, at 10:40 PM, Deneau, Tom <tom.deneau at amd.com<mailto: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<mailto: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<mailto: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<mailto: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