Hello,
On 14.03.2016 12:52, Andrey Belevantsev wrote:
Hello,
The problem here is readonly dependence contexts in selective scheduler.
We're trying to cache the effect of initializing a dependence context
with
remembering that context and setting a readonly bit on it. When first
moving the insn 43 with REG_ARGS_SIZE note through the insn 3 (a
simple eax
set) we also set the last_args_size field of the context. Later, when we
make a copy of insn 43 and try to move it again through insn 3, we
take the
cached dependency context and notice the (fake) dep with last_args_size
insn, which is the old insn 43. Then the assert saying that we should be
able to lift the bookkeeping copy up the same way as we did with the
original insn breaks.
Fixed by the attached patch that makes us notice only deps with the
current
producer insn.
Ok for trunk?
We've discussed the patch with Alexander a bit and decided to take the
different approach. The core issue here is not handling the new
last_args_size field in the readonly contexts. In general, the readonly
context approach, when analyzing an insn with a readonly context, would
create the necessary dependencies with all of the last_ fields but
refrain from modifying those fields. The reason is we need to capture
the effect of only the single producer in the readonly context. Failing
to do so may update the last_ fields with the effect of subsequent
analyses and having the fake dependencies with the insns that got into
those fields instead of having the dependencies with the currently used
producer.
So the right fix here is to guard setting of the last_args_size field
with !deps->readonly test as it is done elsewhere in the sched-deps.c.
In stage 1 we will also want to set the asserts in the sel-sched
dependency hooks (where I have placed early returns in the previous
version of the patch) actually checking that the dependency is always
created with the current producer, and such cases will be caught sooner.
The new patch bootstrapped and tested on x86-64 with selective scheduler
forced enabled with no regressions. It needs the maintainer outside of
sel-sched as we touch sched-deps.c file. Ok for trunk? The test is the
same as in previous patch.
Andrey
2016-03-15 Andrey Belevantsev <abel@ispras.ru>
PR rtl-optimization/69102
* sched-deps.c (sched_analyze_insn): Do not set last_args_size
field
when we have a readonly dependency context.
gcc/
2016-03-14 Andrey Belevantsev <abel@ispras.ru>
PR rtl-optimization/69102
* sel-sched.c (has_dependence_note_dep): Only take into
account dependencies produced by the current producer insn.
(has_dependence_note_mem_dep): Likewise.
testsuite/
2016-03-14 Andrey Belevantsev <abel@ispras.ru>
PR rtl-optimization/69102
* gcc.c-torture/compile/pr69102.c: New test.