This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Need help with PR71976 combine.c::get_last_value returns a wrong result


Tracking this wrong-code bug, I ended up in combine.c::get_last_value() which returns a wrong result. gcc generates wrong code at least with: 4.9 head, 5.2, 6.1 and trunk from today.

Before combine we have:

(insn 43 31 44 2 (set (reg:QI 18 r18)
        (const_int 0 [0])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 44 43 45 2 (set (reg:QI 19 r19 [+1 ])
        (const_int 0 [0])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 45 44 46 2 (set (reg:QI 20 r20 [+2 ])
        (const_int 0 [0])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 46 45 47 2 (set (reg:QI 21 r21 [+3 ])
        (const_int 0 [0])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 47 46 48 2 (set (reg:QI 22 r22 [+4 ])
        (const_int 0 [0])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 48 47 49 2 (set (reg:QI 23 r23 [+5 ])
        (reg:QI 65 [ _3+5 ])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 49 48 50 2 (set (reg:QI 24 r24 [+6 ])
        (reg:QI 66 [ _3+6 ])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 50 49 51 2 (set (reg:QI 25 r25 [+7 ])
        (reg:QI 67 [ _3+7 ])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 51 50 52 2 (set (reg:QI 16 r16)
        (const_int 40 [0x28])) bug-combin.c:29 56 {movqi_insn}
     (nil))
(insn 52 51 61 2 (set (reg:DI 18 r18)
        (ashiftrt:DI (reg:DI 18 r18)
            (reg:QI 16 r16))) bug-combin.c:29 1417 {ashrdi3_insn}
     (expr_list:REG_DEAD (reg:QI 16 r16)
        (nil)))

insn-combine combines 51 -> 52 and then tries to simplify the resulting
 (ashiftrt:DI (reg:DI 18 r18) (const_int 40)) in

simplify_shift_const_1 (code=ASHIFTRT, result_mode=DImode, varop=0x7ffff734ee70, orig_count=40) at ../../../gcc.gnu.org/trunk/gcc/combine.c:10181

note that reg:DI 18 is a hard register. In that function, the following piece of code triggers which turns the shift offset from 40 to 0:

(gdb) p varop
$95 = (rtx) 0x7ffff734ee70
(gdb) pr
(reg:DI 18 r18)

      /* An arithmetic right shift of a quantity known to be -1 or 0
	 is a no-op.  */
      if (code == ASHIFTRT
	  && (num_sign_bit_copies (varop, shift_mode)
	      == GET_MODE_PRECISION (shift_mode)))
	{
	  count = 0;
	  break;
	}

num_sign_bit_copies() eventually calls combine.c::reg_num_sign_bit_copies_for_combine()

which returns const0_rtx because reg 18 is set in insn 43 to const0_rtx. Total outcome is that the right shift of reg:DI 18 is transformed to a no-op move and cancelled out in the remainder.

IIUC get_last_value should return 0 in this case?

  /* If we don't have a value, or if it isn't for this basic block and
     it's either a hard register, set more than once, or it's a live
     at the beginning of the function, return 0.

     Because if it's not live at the beginning of the function then the reg
     is always set before being used (is never used without being set).
     And, if it's set only once, and it's always set before use, then all
     uses must have the same last value, even if it's not from this basic
     block.  */

  if (value == 0
      || (rsp->last_set_label < label_tick_ebb_start
	  && (regno < FIRST_PSEUDO_REGISTER
	      || regno >= reg_n_sets_max
	      || REG_N_SETS (regno) != 1
	      || REGNO_REG_SET_P
		 (DF_LR_IN (ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb), regno))))
    return 0;

value is const_int 0
rsp->last_set_label is 2
label_tick_ebb_start is 1

so that the test "regno < FIRST_PSEUDO_REGISTER" never applies...

It follows

get_last_value_validate (loc=0x7fffffff8ee0, insn=0x7ffff730cc00, tick=2, replace=0) at ../../../gcc.gnu.org/trunk/gcc/combine.c:13062

(gdb) p insn
$5 = (rtx_insn *) 0x7ffff730cc00
(gdb) pr
(insn 43 31 44 2 (set (reg:QI 18 r18)
        (const_int 0 [0])) bug-combin.c:29 56 {movqi_insn}
     (nil))

(gdb) p *loc
$6 = (rtx) 0x7ffff73094f0
(gdb) pr
(const_int 0 [0])

The comment of that function read:

/* Verify that all the registers and memory references mentioned in *LOC are
   still valid.  *LOC was part of a value set in INSN when label_tick was
   equal to TICK.  Return 0 if some are not.  If REPLACE is nonzero, replace
   the invalid references with (clobber (const_int 0)) and return 1.  This
   replacement is useful because we often can get useful information about
   the form of a value (e.g., if it was produced by a shift that always
   produces -1 or 0) even though we don't know exactly what registers it
   was produced from.  */

static int
get_last_value_validate (rtx *loc, rtx_insn *insn, int tick, int replace)
{

which does not make sense if *loc is a const_int because it has no "register" or "memory references".

Which place of get_last_value should handle such sets of hard regs, and how?

Should get_last_value_validate be called for const0_rtx ?

Johann


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]