Re: [PATCH] s390: Add -fsplit-stack support

[Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>]

On 01/29/2016 04:43 PM, Marcin KoÅcielnicki wrote:
> The testsuite with -fsplit-stack already hits all of them, and checking 
> them manually is rather tricky (I don't know if it could be done in 
> target-independent way at all), but I think it'd be reasonable to make 
> assembly testcases calling __morestack for the last two cases, to check 
> if the registers are being preserved, etc.
Sounds good. Thanks!

...
>>> +  if (frame_size <= 0x7fff || (TARGET_EXTIMM && frame_size <= 0xffffffffu))
>> The agfi immediate value is a signed 32 bit integer.  So you can only
>> add up to 2G-1.  I think it would be more readable to write this as:
> 
> We're emitting ALGFI here, which accepts unsigned 32-bit integer.
Ah right. Then it would be:

if (CONST_OK_FOR_K (frame_size) || CONST_OK_FOR_Op (frame_size))

instead.

>>
>> if (CONST_OK_FOR_K (frame_size) || CONST_OK_FOR_Os (frame_size))
>>
>> as in s390_emit_prologue. The Os check will check for TARGET_EXTIMM as well.
> 
> Alright.

...

>> I'm wondering if it is really necessary to expand the call in that
>> two-step approach?! We do the general literal pool handling in
>> s390_reorg because we need all the insn lengths to be finalized before
>> performing the branch/pool splitting loop.  But this shouldn't be necessary
>> in this case.  Would it be possible to expand the call already in
>> emit_prologue phase and get rid of the s390_reorg part?
> 
> There's an internal literal pool involved, which needs to be emitted as 
> one chunk.  Optimizations are also very likely to destroy the sequence: 
> consider the target address that __morestack will call - the control 
> flow change happens in __morestack jump instruction, but the address 
> itself is encoded in one of the pool literals.  Just not worth the risk.
Ok.

...
>>> +	# OK, no stack allocation needed.  We still follow the protocol and
>>> +	# call our caller - it doesn't cost much and makes sure vararg works.
>>> +	# No need to set any registers here - %r0 and %r2-%r6 weren't modified.
>>> +	basr	%r14, %r10		# Call our caller.
>> The comment confuses me.  It somewhat sounds to me like the call
>> wouldn't be really needed but in fact it cannot even remotely work
>> without jumping back to the function body right?!
> 
> Certainly.  __morestack's task is to call the given function entry point 
> once the necessary stack space is established.  In fact, in the no 
> allocation case, a sibling-call would actually be possible, if it 
> weren't for one annoying detail: there are no free GPRs we could use to 
> keep the address of the entry point - %r0 may be used to keep static 
> chain, %r1 may have to be the argument pointer, %r2-%r5 may be used to 
> keep parameters, and %r6-%r15 are callee-saved.
Ok. The comment isn't about no-call vs. call it is about sibcall vs. call - got it.

Bye,

-Andreas-