Memory allocation in HSAIL

Tue May 13 21:49:58 UTC 2014

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