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

Georg-Johann Lay avr@gjlay.de
Mon Jul 25 12:28:00 GMT 2016


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



More information about the Gcc mailing list