Memory allocation in HSAIL

[Deneau, Tom]

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
> >>>
> >