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

[Coleen Phillimore]

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