Unwanted IRA copies via process_reg_shuffles

[Jeff Law <law at redhat dot com>]

I was looking at a regression caused by having ira-reload utilize the 
existing copy detection code in IRA rather than my own and stumbled upon 
this...

Consider this insn prior to IRA:

(insn 72 56 126 8 j.c:744 (parallel [
            (set (reg:SI 110)
                (minus:SI (reg:SI 69 [ ew_u$parts$lsw ])
                    (reg:SI 68 [ ew_u$parts$lsw ])))
            (clobber (reg:CC 17 flags))
        ]) 290 {*subsi_1} (expr_list:REG_DEAD (reg:SI 69 [ 
ew_u$parts$lsw ])
        (expr_list:REG_DEAD (reg:SI 68 [ ew_u$parts$lsw ])
            (expr_list:REG_UNUSED (reg:CC 17 flags)
                (nil)))))

Which matches this pattern in the x86 backend:

(define_insn "*sub<mode>_1"
  [(set (match_operand:SWI 0 "nonimmediate_operand" "=<r>m,<r>")
        (minus:SWI
          (match_operand:SWI 1 "nonimmediate_operand" "0,0")
          (match_operand:SWI 2 "<general_operand>" "<r><i>,<r>m")))
   (clobber (reg:CC FLAGS_REG))]
  "ix86_binary_operator_ok (MINUS, <MODE>mode, operands)"
  "sub{<imodesuffix>}\t{%2, %0|%0, %2}"
  [(set_attr "type" "alu")
   (set_attr "mode" "<MODE>")])


Note carefully that the constraints require operands 0 and 1 to match.  
Operand 2 is not tied to any other operand.  In fact, if operand 2 is 
tied to operand0, then we are guaranteed to generate a reload and muck 
up the code pretty badly.


Now looking at the copies recorded by IRA we have:


  cp0:a0(r95)<->a1(r70)@248:move
  cp1:a4(r69)<->a6(r110)@178:constraint
  cp2:a3(r68)<->a6(r110)@22:shuffle
  cp3:a9(r66)<->a10(r92)@11:shuffle
  cp4:a11(r79)<->a12(r93)@89:move
  cp5:a1(r70)<->a14(r96)@114:constraint
  cp6:a1(r70)<->a13(r97)@114:constraint


Note carefully cp2 which claims a shuffle-copy between r68 and r110.   
ISTM that when trying to assign a hard reg to pseudo r110 that if r68 
has a hard reg, but r69 does not, then pseudo r110 will show a cost 
savings if it is allocated into the same hard reg as pseudo r68.


The problematic code is add_insn_allocno_copies:
   {
      extract_insn (insn);
      for (i = 0; i < recog_data.n_operands; i++)
        {
          operand = recog_data.operand[i];
          if (REG_SUBREG_P (operand)
&& find_reg_note (insn, REG_DEAD,
                                REG_P (operand)
                                ? operand : SUBREG_REG (operand)) != 
NULL_RTX)
            {
              str = recog_data.constraints[i];
              while (*str == ' ' || *str == '\t')
                str++;
              bound_p = false;
              for (j = 0, commut_p = false; j < 2; j++, commut_p = true)
                if ((dup = get_dup (i, commut_p)) != NULL_RTX
&& REG_SUBREG_P (dup)
&& process_regs_for_copy (operand, dup, true,
                                              NULL_RTX, freq))
                  bound_p = true;
              if (bound_p)
                continue;
              /* If an operand dies, prefer its hard register for the
                 output operands by decreasing the hard register cost
                 or creating the corresponding allocno copies.  The
                 cost will not correspond to a real move insn cost, so
                 make the frequency smaller.  */
              process_reg_shuffles (operand, i, freq < 8 ? 1 : freq / 8);
            }
        }


With r68 dying and not bound to an another operand, we create a 
reg-shuffle copy to encourage tying r68 to the output operand.  Not good.

ISTM that if an output is already bound to some input that we should not 
be recording a copy between an unbound dying input and the bound output.

You can play with the attached (meaningless) testcase -O2 -m32 -fPIC.  
You won't see code quality regressions due to this problem on this 
testcase with the mainline sources, but it should be enough to trigger 
the bogus copy.

Thoughts?

Jeff

Attachment: j.c
Description: Text document