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