[Bug rtl-optimization/60043] New: -fschedule-insns2 breaks anti-dependency

[Bug rtl-optimization/60043] New: -fschedule-insns2 breaks anti-dependency

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60043

            Bug ID: 60043
           Summary: -fschedule-insns2 breaks anti-dependency
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
            Target: x86_64-*-*

int foo (long long *a, short *b, int n)
{
  *a = (long long)(n * 100);

  return (*b) + 1000;
}

produces at -O2

foo:
.LFB0:
        .cfi_startproc
        imull   $100, %edx, %edx
        movswl  (%rsi), %eax
        movslq  %edx, %rdx
        movq    %rdx, (%rdi)
        addl    $1000, %eax
        ret

fixed by -fno-schedule-insns2.

[Bug rtl-optimization/60043] -fschedule-insns2 breaks anti-dependency

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60043

Andrey Belevantsev <abel at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |abel at gcc dot gnu.org

--- Comment #1 from Andrey Belevantsev <abel at gcc dot gnu.org> ---
I don't follow the reasoning of this example and the original ML thread.  The
load of *b follows the store to *a, thus the scheduler is checking for the
presence of the _true_ dependence between them:

gcc/sched-deps.c:
2660                 if (true_dependence (XEXP (pending_mem, 0), VOIDmode, t)
2661                     && ! sched_insns_conditions_mutex_p (insn,
2662                                                          XEXP (pending,
0)))
2663                   note_mem_dep (t, XEXP (pending_mem, 0), XEXP (pending,
0),
2664                                 sched_deps_info->generate_spec_deps
2665                                 ? BEGIN_DATA | DEP_TRUE : DEP_TRUE);

which does not exist because the mems have different alias sets.  But you have
agreed that TBAA can be used for true dependences in the ML thread, no?  What
is then required from the scheduler?

[Bug rtl-optimization/60043] -fschedule-insns2 breaks anti-dependency

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60043

--- Comment #2 from Andrey Belevantsev <abel at gcc dot gnu.org> ---
And indeed, if we change the test case to

int foo (long long *a, short *b, int n)
{
  int k = *b + 1000;
  *a = (long long) (n * 100);

  return k;
}

then we get the desired anti-dependency because alias.c:write_dependence will
dispatch to rtx_refs_may_alias_p with tbaa_p = false.  So I don't see an issue
here wrt the ML thread discussion.

[Bug rtl-optimization/60043] -fschedule-insns2 breaks anti-dependency

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60043

--- Comment #3 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 4 Feb 2014, abel at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60043
> 
> Andrey Belevantsev <abel at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |abel at gcc dot gnu.org
> 
> --- Comment #1 from Andrey Belevantsev <abel at gcc dot gnu.org> ---
> I don't follow the reasoning of this example and the original ML thread.  The
> load of *b follows the store to *a, thus the scheduler is checking for the
> presence of the _true_ dependence between them:
> 
> gcc/sched-deps.c:
> 2660                 if (true_dependence (XEXP (pending_mem, 0), VOIDmode, t)
> 2661                     && ! sched_insns_conditions_mutex_p (insn,
> 2662                                                          XEXP (pending,
> 0)))
> 2663                   note_mem_dep (t, XEXP (pending_mem, 0), XEXP (pending,
> 0),
> 2664                                 sched_deps_info->generate_spec_deps
> 2665                                 ? BEGIN_DATA | DEP_TRUE : DEP_TRUE);
> 
> which does not exist because the mems have different alias sets.  But you have
> agreed that TBAA can be used for true dependences in the ML thread, no?  What
> is then required from the scheduler?

Yes, TBAA can be used for true dependences - but a true dependence is
read-after-write.  Here we have an anti-dependence, write-after-read.
When the scheduler wants to exchange two mems then it needs to use
the predicate that is correct _before_ the transform, not after.

Thus in the above code it seems that it does not check whether the
write from pending[_mem] is before or after the read in 't' - the
used predicate needs to change dependent on the order of the insns
(or conservatively assume an anti-dependence and thus disable TBAA).

[Bug rtl-optimization/60043] -fschedule-insns2 breaks anti-dependency

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60043

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, oops.  The testcase has a true dependence ... ;)  Still, isn't the
code in sched_analyze_2 wrong?  Or are pending_mems all before 't'?

[Bug rtl-optimization/60043] -fschedule-insns2 breaks anti-dependency

[abel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60043

--- Comment #5 from Andrey Belevantsev <abel at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #4)
> Hmm, oops.  The testcase has a true dependence ... ;)  Still, isn't the
> code in sched_analyze_2 wrong?  Or are pending_mems all before 't'?

Yes (to second question :).  The analysis code roughly works like this
(sched_analyze is the entry point):

- process insns one by one in a bb/ebb doing two things in parallel:

  o mark reads/writes to registers, writes/reads to mems of this insn in
various fields of the deps context; and
  o find dependencies between this insn reads/writes of registers/memory and
previously noted reads/writes of the same register or memory from the deps
context.

So the pending_mem read/write will always happen before the currently
processing insn mem read/write.