Fix bootstrap with -mno-accumulate-outgoing-args

This time with the patch...

On 02/05/2014 07:40 AM, Richard Henderson wrote:
> On 02/04/2014 09:35 AM, Jan Hubicka wrote:
>>> On 02/04/2014 08:48 AM, Jan Hubicka wrote:
>>>> How things are supposed to work in this case? So perhaps we need scheduler to
>>>> understand and move around the ARGS_SIZE note?
>>>
>>> I believe we do need to have the scheduler move the notes around.
>>
>> If we need notes on non-stack adjusting insns as you seem to show in your testcase,
>> I guess it is the only way around.  Still combine stack adjust may be smart enough
>> to not produce redundant note in the case of go's ICE.
>>
>> I am not terribly familiar with the code, will you look into it or shall I give it a try?
> 
> I had a brief look at find_modifiable_mems, which seems to be the only kind of
> schedule-time dependency breaking that we do.  I started writing some new code
> that would handle REG_ARGS_SIZE, but then considered that wasn't terribly
> appropriate for stage4.
> 
> So I've left off with just the dependency links between the insns.  We'd need
> these for the schedule-time adjustments anyway, and with them in place the
> scheduler does not re-order the insns in a way that causes problems.
> 
> Testing for x86_64 has finished; i686 is nearly done.  I suppose the only
> question I have is if anyone disagrees that OUTPUT is incorrect as the
> dependency type.
> 
> 
> r~
>
* combine-stack-adj.c: Revert r206943.
	* sched-int.h (struct deps_desc): Add last_args_size.
	* sched-deps.c (init_deps): Initialize it.
	(sched_analyze_insn): Add OUTPUT dependencies between insns that
	contain REG_ARGS_SIZE notes.

> > I had a brief look at find_modifiable_mems, which seems to be the only kind of
> > schedule-time dependency breaking that we do.  I started writing some new code
> > that would handle REG_ARGS_SIZE, but then considered that wasn't terribly
> > appropriate for stage4.
> > 
> > So I've left off with just the dependency links between the insns.  We'd need
> > these for the schedule-time adjustments anyway, and with them in place the
> > scheduler does not re-order the insns in a way that causes problems.
> > 
> > Testing for x86_64 has finished; i686 is nearly done.  I suppose the only
> > question I have is if anyone disagrees that OUTPUT is incorrect as the
> > dependency type.

From i386  perspective, this seems like resonable compromise for now.
I did some re-testing of scheduler recently for generic.
Call sequences is one of few places where scheduling matters (in addition to
tight loops), but I do not think we will lose measurable % of perofmrance by
disabling scheduling here.

Thanks!
Honza
> > 
> > 
> > r~
> > 
> 

> 	* combine-stack-adj.c: Revert r206943.
> 	* sched-int.h (struct deps_desc): Add last_args_size.
> 	* sched-deps.c (init_deps): Initialize it.
> 	(sched_analyze_insn): Add OUTPUT dependencies between insns that
> 	contain REG_ARGS_SIZE notes.
> 
> 
> diff --git a/gcc/combine-stack-adj.c b/gcc/combine-stack-adj.c
> index c591c60..69fd5ea 100644
> --- a/gcc/combine-stack-adj.c
> +++ b/gcc/combine-stack-adj.c
> @@ -567,7 +567,6 @@ combine_stack_adjustments_for_block (basic_block bb)
>  	      && try_apply_stack_adjustment (insn, reflist, 0,
>  					     -last_sp_adjust))
>  	    {
> -	      rtx note;
>  	      if (last2_sp_set)
>  		maybe_move_args_size_note (last2_sp_set, last_sp_set, false);
>  	      else
> @@ -577,11 +576,6 @@ combine_stack_adjustments_for_block (basic_block bb)
>  	      reflist = NULL;
>  	      last_sp_set = NULL_RTX;
>  	      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;
>  	    }
>  	}
> diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
> index 7efc937..efc4223 100644
> --- a/gcc/sched-deps.c
> +++ b/gcc/sched-deps.c
> @@ -3470,6 +3470,15 @@ sched_analyze_insn (struct deps_desc *deps, rtx x, rtx insn)
>              change_spec_dep_to_hard (sd_it);
>          }
>      }
> +
> +  /* We do not yet have code to adjust REG_ARGS_SIZE, therefore we must
> +     honor their original ordering.  */
> +  if (find_reg_note (insn, REG_ARGS_SIZE, NULL))
> +    {
> +      if (deps->last_args_size)
> +	add_dependence (insn, deps->last_args_size, REG_DEP_OUTPUT);
> +      deps->last_args_size = insn;
> +    }
>  }
>  
>  /* Return TRUE if INSN might not always return normally (e.g. call exit,
> @@ -3876,6 +3885,7 @@ init_deps (struct deps_desc *deps, bool lazy_reg_last)
>    deps->sched_before_next_jump = 0;
>    deps->in_post_call_group_p = not_post_call;
>    deps->last_debug_insn = 0;
> +  deps->last_args_size = 0;
>    deps->last_reg_pending_barrier = NOT_A_BARRIER;
>    deps->readonly = 0;
>  }
> diff --git a/gcc/sched-int.h b/gcc/sched-int.h
> index 3b1106f..2cec624 100644
> --- a/gcc/sched-int.h
> +++ b/gcc/sched-int.h
> @@ -539,6 +539,9 @@ struct deps_desc
>    /* The last debug insn we've seen.  */
>    rtx last_debug_insn;
>  
> +  /* The last insn bearing REG_ARGS_SIZE that we've seen.  */
> +  rtx last_args_size;
> +
>    /* The maximum register number for the following arrays.  Before reload
>       this is max_reg_num; after reload it is FIRST_PSEUDO_REGISTER.  */
>    int max_reg;