AIX regression due to DFA scheduler merge

[Vladimir Makarov]

law@redhat.com wrote:
> 
>  In message <3CF7B981.6020D604@redhat.com>, Vladimir Makarov writes:
>  > Yes, you are absolutely right.  This is a latent scheduler bug.  And we
>  > wre lucky not to meet the bug.  The scheduler ignores REG_NO_CONFLICT at
>  > all.  So we have two libcall sequences
>  >
>  > (set (subreg:SI ((reg:DI 122) 0) ... (expr_list:REG_NO_CONFLICT (reg:DI
>  > 117) ...
>  > (set (subreg:SI ((reg:DI 122) 4) ... (expr_list:REG_NO_CONFLICT (reg:DI
>  > 117) ...
>  >
>  >  ...
>  >
>  > (set (subreg:SI ((reg:DI 123) 0) ... (expr_list:REG_NO_CONFLICT (reg:DI
>  > 117) ...
>  > (set (subreg:SI ((reg:DI 123) 4) ... (expr_list:REG_NO_CONFLICT (reg:DI
>  > 117) ...
>  >
>  > Because the scheduler ignores REG_NO_CONFLICT, there are no dependencies
>  > between insns in two groups and the scheduler intermixes them.  Register
>  > allocator takes the REG_NO_CONFLICT into account and assigns the same
>  > register to pseudos 122 and 123.
> Presumably this mixing of code from two different libcalls is happening
> pre-register allocation?  If so, there's code in the register allocator to
> detect foreign insns in a libcall sequence containing REG_NO_CONFLICT notes.
> Presumably that code is not functioning properly.
> 
> If we look at local-alloc.c::no_conflict_p we see:
> 
>   for (p = NEXT_INSN (insn); p && p != last; p = NEXT_INSN (p))
>     if (INSN_P (p))
>       {
>         if (find_reg_note (p, REG_DEAD, r1))
>           ok = 1;
> 
>         /* There must be a REG_NO_CONFLICT note on every insn, otherwise
>            some earlier optimization pass has inserted instructions into
>            the sequence, and it is not safe to perform this optimization.
>            Note that emit_no_conflict_block always ensures that this is
>            true when these sequences are created.  */
>         if (! find_reg_note (p, REG_NO_CONFLICT, r1))
>           return 0;
>       }
> 
> It seems to me that if the foreign insn has a REG_NO_CONFLICT note (because
> it was in some other libcall block), then we won't detect it as a foreign
> insn and, well, we've seen what happens.
> 
> So there's another way to attack this problem -- make detection of foreign
> insns in libcall/noconflict sequences more accurate.
> 

yes, I think it is simpler than processing REG_NO_CONFLICT in the
scheduler.

What we need is to check additionally that this is not a nested libcall
because we have originally


LIBCALL 1
insn1
insn2
RETVAL  1

LIBCALL 2
insn3
insn4
RETVAL  2

and after 1st insn scheduling we have

LIBCALL 2
LIBCALL 1
insn3
insn1
insn2
insn4
RETVAL 1
RETVAL 2

For LIBCALL1 function no_conflict_p still returns TRUE.  Actually I
don't know what solution to choose.  The both of them have advantages
and disadvantages:

   Fix in n_conflict_p is easier and permits insn scheduling of two
libcalls which, with my point of view, is more important than
opportunity in register allocation for processors needing good insn
scheduling.

   On the other hand, we have unclear rtx code (nested libcalls and no
dependencies in rtx).  Checking the nested calls probably is more
expensive.  The fix in the scheduler is better with this point of view.

   I'd like to have your opinions which fix is better.

Vlad

> Now what I find particularly interesting about this is that it *may* tie
> into the regressions we've seen when trying to schedule a libcall group
> as a single entity.  It's a slight stretch, but I can envision ways this
> latent bug could be interacting badly when the libcall group scheduling
> patch.