Memory allocation in HSAIL

Tue May 13 22:01:07 UTC 2014

Thanks.  I will push this.

On May 13, 2014, at 2:49 PM, Deneau, Tom <tom.deneau at amd.com> wrote:

> Christian, Doug --
>  
> Attached is the small patch to take care of this…
>  
> -- Tom
>  
> From: Christian Thalinger [mailto:christian.thalinger at oracle.com] 
> Sent: Tuesday, May 13, 2014 3:46 PM
> To: Deneau, Tom
> Cc: Doug Simon; graal-dev at openjdk.java.net
> Subject: Re: Memory allocation in HSAIL
>  
> Oh.  I missed the push of these:
>      @HotSpotVMConstant(name = "sizeof(HSAILFrame)") @Stable public int hsailFrameHeaderSize;
>      @HotSpotVMConstant(name = "sizeof(Hsail::HSAILKernelDeoptimization)") @Stable public int hsailKernelDeoptimizationHeaderSize;
> -    @HotSpotVMField(name = "Hsail::HSAILDeoptimizationInfo::_deopt_save_states[0]", type = "Hsail::HSAILKernelDeoptimization", get = HotSpotVMField.Type.OFFSET) @Stable public int hsailSaveStatesOffset0;
> +    @HotSpotVMConstant(name = "sizeof(Hsail::HSAILDeoptimizationInfo)") @Stable public int hsailDeoptimizationInfoHeaderSize;
>  
> sizeof types should be handled differently with HotSpotVMType like:
>  
>     @HotSpotVMType(name = "vtableEntry", get = HotSpotVMType.Type.SIZE) @Stable public int vtableEntrySize;
>  
> Tom, can you change that?
>  
> On May 9, 2014, at 1: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
>  
>  
>  
>  
> <hotspotvmtype-for-sizeof.patch>