Re: Fix bootstrap with -mno-accumulate-outgoing-args

[Jan Hubicka <hubicka at ucw dot cz>]

> On 01/01/2014 06:23 AM, Jan Hubicka wrote:
> >  	      last_sp_adjust = 0;
> > +	      /* We no longer adjust stack size.  Whoever adjusted it earlier
> > +		 hopefully got the note right.  */
> > +	      note = find_reg_note (insn, REG_ARGS_SIZE, NULL_RTX);
> > +	      if (note)
> > +		remove_note (insn, note);
> >  	      continue;
> 
> This is a totally unfounded hope, by the way.  I'm certain that I tried this

Sorry to hear that - that assumption seemed right in the testcase I loked into.

> the first time around, with similarly poor results.
> 
> See the g++.dg/opt/pr52727.C test case that fails with this patch.  I can only
> assume you didn't examine i686 test results when you committed this?
> 
> The problem is that the last adjustment _did_ apply the correct note
> adjustment, for the stack at that time.  But neither moving the note nor
> dropping the note is correct.
> 
> Consider
> 
> 					// args_size 4
> 	add	$4, %esp		// args_size 0
> 	push	%eax			// no note; fpu, not call related
> 
> transformed by csa to
> 
> 	mov	%eax, (%esp)
> 
> If we move the note from the add to the mov, as we did prior to your patch, we
> retain the essence of the pop, that we're no longer queueing arguments.  But if
> we drop the note, as we do with your patch, then we still believe that we have
> 4 bytes of arguments on the stack.
> 
> You say a bootstrap with Go was the testcase that sent you in this direction?
> I'll have a look...

Yes, if you remove the patch you should get ICE during go x86_64 bootstrap with
generic tuning.

I tried to describe the situation in original mail and the PR - the sequence was
same as you describe here, but comming from two consecutive calls, so args_size
was 8, then reduced to 0 by add and then increased to 8 by push.

I see in the testcase the psh comes from splitter - that is something I did
not consider indded. I am sorry for that.
How things are supposed to work in this case? So perhaps we need scheduler to
understand and move around the ARGS_SIZE note?

Honza