RFR(S): 8017313: PPC64 (part 6): cppInterpreter: stack handling improvements

Wed Jun 26 14:25:00 PDT 2013

I updated bug report to remove cppInterpreter from synopsis and description.

Do we have agreement on these changes?

Will runtime group do changes in our code to use new functionality?
If not, we should push it into staging repo because I don't see other 
reasons to push it into main sources now.

Thanks,
Vladimir

On 6/26/13 6:28 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> I added shared initialization code of the new field.
> I also moved the code from thread to JavaThread, as only
> there overflow checks happen.
> http://cr.openjdk.java.net/~goetz/webrevs/8017313-stack-2/
>
> Do you want to push this to some main repository (if you want
> to use this on other platforms) or should I push it into the
> ppc64 staging repo once tested and finally reviewed?
>
> Best regards,
>    Goetz.
>
> From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com]
> Sent: Dienstag, 25. Juni 2013 16:23
> To: Lindenmaier, Goetz
> Cc: hotspot-dev at openjdk.java.net; David Holmes
> Subject: Re: RFR(S): 8017313: PPC64 (part 6): cppInterpreter: stack handling improvements
>
>
> On 06/25/2013 10:07 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> that's something I don't get:  x86 does
>    // Use the maximum number of pages we might bang.
>    const int max_pages = StackShadowPages > (StackRedPages+StackYellowPages) ? StackShadowPages :
>                                                                                (StackRedPages+StackYellowPages)
> and sparc
>      __ set( (StackRedPages+StackYellowPages) * page_size, Rscratch2 );
> in generate_stack_overflow_check().
>
> I don't know why these are different, except that they are coded separately and probably last touched at different times.
>
>
>
> And we use the sum of all three.  This is because we want to use the
> shadow space for exception handling, so we must overflow before we
> run into the shadow space.
>
> Yes, we don't want to check that we are not pushing stack frames into the shadow space here.
>
>
>
> All these different values could be set in the os_cpu file.
> But maybe they only differ because there is no shared coding
> yet?  And they should be harmonized?
>
> They absolutely should be harmonized, which would  be good if the calculation that it uses is shared and the assembly code is simplified.   We can do that after your change.
>
>
>
> Bertrand, recomputing stack_base seems a good idea to me.
> But this would require that all platforms are adapted before
> _stack_base is removed.
>
> We could do that when we fix the other platforms to use stack_overflow_limit().   The extra field in Thread is worth it.   There are a lot of fields in Thread that we can remove (our out-pointer because they're never used) instead.
>
> Coleen
>
>
>
> Best regards,
> Goetz.
>
>
>
> From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com]
> Sent: Dienstag, 25. Juni 2013 15:34
> To: Lindenmaier, Goetz
> Cc: hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>; David Holmes
> Subject: Re: RFR(S): 8017313: PPC64 (part 6): cppInterpreter: stack handling improvements
>
>
> Yes, I think this would be useful for the other platforms to simplify the stack overflow limit checking in the interpreter assembly code.
>
>        100   // Initialize the memory stack limit.
>
>        101   address mem_stk_limit = thread->stack_base() - thread->stack_size() +
>
>        102     ((StackShadowPages + StackYellowPages + StackRedPages) * os::vm_page_size());
>
>        103
>
>        104   thread->set_memory_stack_limit(mem_stk_limit);
>
>
>
>
> My other comment was to move 101-102 into the function set_stack_overflow_limit() since nothing on those lines depends on the platform and the values used to compute this are owned by class Thread anyway.
>
> thanks,
> Coleen
> On 06/25/2013 09:27 AM, Lindenmaier, Goetz wrote:
>
> Hi Colleen, David,
>
>
>
> I'll rename the methods to stack_overflow_limit and update the webrev,
>
> I like that name better too.
>
>
>
> We set it in os::initialize_thread():
>
> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/2a6fd169b0b7/src/os_cpu/linux_ppc/vm/os_linux_ppc.cpp
>
> and we use it here (line427):
>
> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/2a6fd169b0b7/src/cpu/ppc/vm/cppInterpreter_ppc.cpp
>
> In that files the method is called [set_]memory_stack_limit().
>
>
>
> Actually, it's got nothing to do with the cppInterpreter.  I first thought so,
>
> because we only used it with the cppInterpreter when I moved this from
>
> our VM to the port (a while ago now).  But look at the code in
>
> void InterpreterGenerator::generate_stack_overflow_check(void) on x86 that
>
> computes the stack bound in rax ... that's not really simple, and requires
>
> two loads (stack_base, stack_size).
>
>
>
> The difference between cppInterpreter and template interpreter is that
>
> we always check against the stack_limit in the cppInterpreter, but the
>
> templateInterpreter first checks whether the stack is < 1 page, thus
>
> experiencing less overhead from the lengthy coding.
>
>
>
> Best regards,
>
>    Goetz
>
>
>
>
>
> -----Original Message-----
>
> From: hotspot-dev-bounces at openjdk.java.net<mailto:hotspot-dev-bounces at openjdk.java.net> [mailto:hotspot-dev-bounces at openjdk.java.net] On Behalf Of Coleen Phillimore
>
> Sent: Dienstag, 25. Juni 2013 14:49
>
> To: hotspot-dev at openjdk.java.net<mailto:hotspot-dev at openjdk.java.net>
>
> Subject: Re: RFR(S): 8017313: PPC64 (part 6): cppInterpreter: stack handling improvements
>
>
>
>
>
> Which stack limit is this?   Does it subtract off the red and yellow
>
> zones?   Where do you set it?  Yes, it would simplify the assembler code
>
> in the interpreter entries to use this.
>
>
>
> It might be better to rename it though if it's the usable stack
>
> (subtracting red and yellow zones) and maybe compute the value inside of
>
> Thread class (so you see it always subtracting the red and yellow
>
> zones).   It's sort of ambiguous that this is happening. How about
>
> stack_overflow_limit()?
>
>
>
> Thanks,
>
> Coleen
>
>
>
> On 06/25/2013 08:44 AM, Lindenmaier, Goetz wrote:
>
> Hi,
>
>
>
> We precompute the stack limit for overflow checks and save this
>
> limit in the thread.  This simplifies the assembler coding checking
>
> the stack overflow. This change contains the necessary shared
>
> functionality. It can easily be ported to other platforms, too.
>
> http://cr.openjdk.java.net/~goetz/webrevs/8017313-stack/<http://cr.openjdk.java.net/%7Egoetz/webrevs/8017313-stack/>
>
>
>
> Please review this change.
>
>
>
> Best regards,
>
>     Goetz.
>
>
>
> PS: I removed some parts of patch 0006 because I found a better
>
> platform-only solution.
>
>
>
>