Memory allocation in HSAIL

Tue May 13 13:49:56 UTC 2014

I’ve integrated these changes with the following small modification:

- The initialization and deletion of _never_ran_array is now in HSAILDeoptimizationInfo constructor and destructor.

-Doug

On May 9, 2014, at 10:38 PM, Deneau, Tom <tom.deneau at amd.com> wrote:

> Doug --
> 
> I found a bug introduced in the recent hsail-deopt-windows-build-fixes webrev (which won't show up in the junits, it requires the maximum # of concurrent workitems to actually deopt ).
> 
> http://cr.openjdk.java.net/~tdeneau/graal-webrevs/webrev-hsail-deopt-savearea-cleanup
> 
> In addition, I tried implementing a few more of your suggestions.
> 
>   * I didn't see a good way to get rid of the operator new for the outer HSAILDeoptimizationInfo.
>     I guess I could have hidden it behind a new_HSAILDeoptimizationInfo static routine.
>     (I noticed that new_nmethod eventually called operator new for nmethod)
> 
>   * I did remove the ResourceObj declaration for HSAILDeoptimizationInfo since as you said it didn't
>     make sense when we were allocating from the heap.
> 
>   * cleaned up some NPEs when UseHSAILDeoptimization was turned off.  Also marked some junit tests not
>     to run in that case.  We still get some failures if we run the junits with UseHSAILDeoptimization
>     off.  I understand these, they are based on the wide net we cast to decide if allocation is being
>     used for a particular graph.  See HSAILHotSpotBackend.java line 491. 
>     Couldn't think of a quick way to fix this but we should.
> 
>   * some other small cleanups
> 
>   * I didn't see how to use the CodeBlob::allocation_size() so I didn't stick that in.  I did make
>     the HSAILDeoptimizationInfo end in a pointer so that the end of the header/ start of the save
>     area was 8-byte aligned.
> 
> -- Tom
> 
> 
>> -----Original Message-----
>> From: Doug Simon [mailto:doug.simon at oracle.com]
>> Sent: Wednesday, May 07, 2014 6:27 AM
>> To: Deneau, Tom
>> Cc: Gilles Duboscq; Christian Wirth
>> Subject: Re: Memory allocation in HSAIL
>> 
>> I'm pushing these changes now...
>> 
>> On May 7, 2014, at 12:22 AM, Deneau, Tom <tom.deneau at amd.com> wrote:
>> 
>>> OK --
>>> 
>>> http://cr.openjdk.java.net/~tdeneau/graal-webrevs/webrev-hsail-deopt-w
>>> indows-build-fixes/webrev/
>>> 
>>> is a small step in the direction below, mainly to solve the windows
>> build issue.
>>> It still passes the junits on linux.
>>> 
>>> I actually started going a little beyond the specific windows build
>>> issue but decided to postpone those fixes until later.
>>> 
>>> -- Tom
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Doug Simon [mailto:doug.simon at oracle.com]
>>>> Sent: Tuesday, May 06, 2014 12:56 AM
>>>> To: Deneau, Tom
>>>> Cc: Gilles Duboscq; Christian Wirth
>>>> Subject: Re: Memory allocation in HSAIL
>>>> 
>>>> 
>>>> On May 6, 2014, at 1:15 AM, Deneau, Tom <tom.deneau at amd.com> wrote:
>>>> 
>>>>> Doug --
>>>>> 
>>>>> I've been tied up in meetings today...
>>>>> Not sure what your timeframe is, but I can send you a solution that
>>>> builds on windows.
>>>> 
>>>> That would be great.
>>>> 
>>>>> But it doesn't address some of the other issues below, including how
>>>> the HSAILDeoptimizationInfo is allocated.
>>>> 
>>>> Ok - that is not urgent.
>>>> 
>>>>> I don't currently have a way to actually run the hsail backend on
>>>> windows but I can test building there and running elsewhere.
>>>> 
>>>> That's fine. The main thing is to have the build work on Windows.
>>>> 
>>>> Thanks.
>>>> 
>>>> -Doug
>>>> 
>>>>>> -----Original Message-----
>>>>>> From: Doug Simon [mailto:doug.simon at oracle.com]
>>>>>> Sent: Monday, May 05, 2014 3:52 PM
>>>>>> To: Deneau, Tom
>>>>>> Cc: Gilles Duboscq; Christian Wirth
>>>>>> Subject: Re: Memory allocation in HSAIL
>>>>>> 
>>>>>> Something like the pattern in nmethod::new_nmethod should work.
>>>>>> 
>>>>>> -Doug
>>>>>> 
>>>>>> On May 5, 2014, at 10:42 PM, Deneau, Tom <tom.deneau at amd.com>
>> wrote:
>>>>>> 
>>>>>>> Doug --
>>>>>>> 
>>>>>>> Can you point me to an example of how you want the
>>>>>> HSAILDeoptimizationInfo allocation to be structured?
>>>>>>> In particular, for invoking the new operator for ResourceObj.
>>>>>>> 
>>>>>>> -- Tom
>>>>>>> 
>>>>>>> 
>>>>>>>> -----Original Message-----
>>>>>>>> From: Doug Simon [mailto:doug.simon at oracle.com]
>>>>>>>> Sent: Monday, May 05, 2014 5:51 AM
>>>>>>>> To: Deneau, Tom
>>>>>>>> Cc: Gilles Duboscq; Christian Wirth
>>>>>>>> Subject: Memory allocation in HSAIL
>>>>>>>> 
>>>>>>>> Hi Tom,
>>>>>>>> 
>>>>>>>> The way memory is currently allocated for HSAIL deoptimization
>>>>>>>> info departs somewhat from the way HotSpot manages variable
>>>>>>>> size/embedded data structures. One immediate side effect of this
>>>>>>>> is that your recent changes broke the Windows build with the
>>>>>>>> following compiler
>>>>>> warnings:
>>>>>>>> 
>>>>>>>> \src\gpu\hsail\vm\gpu_hsail_Frame.hpp(15): error C2220: warning
>>>>>>>> treated as error - no 'object' file generated [c:\oracl
>>>>>>>> \src\gpu\hsail\vm\gpu_hsail_Frame.hpp(15): warning C4200:
>>>>>>>> nonstandard extension used : zero-sized array in struct/union
>>>>>>>> 
>>>>>>>> Gilles and I have looked at the code and have the following
>>>>>>>> recommendations:
>>>>>>>> 
>>>>>>>> Since HSAILDeoptimizationInfo subclasses ResourceObj, it should
>>>>>>>> not override the new operator. Instead, at the allocation site it
>>>>>>>> should do whatever size computation is necessary (probably in a
>>>>>>>> static method of
>>>>>>>> HSAILDeoptimizationInfo) and call one of the existing new
>>>>>>>> operators defined by ResourceObj. At is currently stands, it's
>>>>>>>> strange to have a ResourceObj subclass that allocates directly
>>>>>>>> from the C
>>>> heap.
>>>>>>>> 
>>>>>>>> The bytesPerSaveArea is computed information that does not need
>>>>>>>> to be stored in a _bytesPerSaveArea. Same goes for _deopt_span.
>>>>>>>> 
>>>>>>>> Since both HSAILKernelDeoptimization and HSAILFrame are really
>>>>>>>> overlays on a HSAILDeoptimizationInfo, they should both use
>>>>>> VALUE_OBJ_CLASS_SPEC.
>>>>>>>> For example:
>>>>>>>> 
>>>>>>>> class HSAILKernelDeoptimization VALUE_OBJ_CLASS_SPEC { ... }
>>>>>>>> 
>>>>>>>> The _num_s_regs, _num_d_regs and _num_stack_slots value are fixed
>>>>>>>> for a given HSAILDeoptimizationInfo and do not need to be stored
>>>>>>>> in each HSAILFrame (I can't even see where they are currently
>>>> initialized).
>>>>>>>> 
>>>>>>>> You should probably think about alignment for the objects
>>>>>>>> embedded in a HSAILDeoptimizationInfo. See
>>>>>>>> CodeBlob::allocation_size() for inspiration.
>>>>>>>> 
>>>>>>>> The HSAILFrame::_save_area and
>>>>>>>> HSAILDeoptimizationInfo::_deopt_save_states fields should go
>> away.
>>>>>>>> Accessing them should be done like the access to the objects
>>>>>>>> embedded in an nmethod (e.g. nmethod::scopes_pcs_begin()).
>>>>>>>> 
>>>>>>>> The highest priority is to fix the Windows build.
>>>>>>>> 
>>>>>>>> -Doug
>>>>> 
>>> 
> 