This is the mail archive of the gcc-patches@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]

Re: [patch] Fix PR rtl-optimization/58831


On 10/24/13 15:28, Eric Botcazou wrote:
This is a very old bug in the alias.c machinery, which is exposed during
the sched2 pass.  We have in .split4:

(insn 23 22 24 4 (set (mem/f:DI (reg/v/f:DI 4 si [orig:90 p2 ] [90]) [3
*p2_9(D)+0 S8 A64])
         (symbol_ref:DI ("d")  <var_decl 0x7ffff6e42720 d>)) pr58831-1.c:12 85
{*movdi_internal}
      (expr_list:REG_DEAD (reg/v/f:DI 4 si [orig:90 p2 ] [90])
         (nil)))

[...]

(insn 61 27 62 6 (clobber (reg:DI 4 si)) -1
      (nil))

(insn/f 62 61 56 6 (parallel [
             (set (mem:DI (pre_dec:DI (reg/f:DI 7 sp)) [0 S8 A8])
                 (reg:DI 4 si))
             (clobber (mem:BLK (scratch) [0 A8]))
         ]) pr58831-1.c:8 73 {*pushdi2_prologue}
      (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:DI 7 sp)
             (plus:DI (reg/f:DI 7 sp)
                 (const_int -8 [0xfffffffffffffff8])))
         (nil)))

[...]

(insn 30 29 31 6 (set (reg:DI 4 si)
         (symbol_ref/f:DI ("*.LC0") [flags 0x2]  <var_decl 0x7ffff6e72098
*.LC0>)) pr58831-1.c:14 85 {*movdi_internal}
      (nil))

si is the 2nd argument register on x86-64 so it's live on entry.  The problem
is that the clobber at insn 61 wipes out the value recorded for si:

       /* A CLOBBER wipes out any old value but does not prevent a previously
	 unset register from acquiring a base address (i.e. reg_seen is not
	 set).  */
       if (GET_CODE (set) == CLOBBER)
	{
	  new_reg_base_value[regno] = 0;
	  return;
	}

by init_alias_target:

   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
     /* Check whether this register can hold an incoming pointer
        argument.  FUNCTION_ARG_REGNO_P tests outgoing register
        numbers, so translate if necessary due to register windows.  */
     if (FUNCTION_ARG_REGNO_P (OUTGOING_REGNO (i))
	&& HARD_REGNO_MODE_OK (i, Pmode))
       static_reg_base_value[i] = arg_base_value;

and later copied by init_alias_analysis:

       /* Mark all hard registers which may contain an address.
	 The stack, frame and argument pointers may contain an address.
	 An argument register which can hold a Pmode value may contain
	 an address even if it is not in BASE_REGS.

	 The address expression is VOIDmode for an argument and
	 Pmode for other registers.  */

       memcpy (new_reg_base_value, static_reg_base_value,
	      FIRST_PSEUDO_REGISTER * sizeof (rtx));

and that nothing indicates that there was a valid value before the clobber, so
the machinery wrongly records the set at insn 30.  I think the fix is just to
set reg_seen[N] if static_reg_base_value[N] is non-null in the copy above made
by init_alias_analysis.  Tested on x86_64-suse-linux.

Jeff, it's old code of yours, are you OK with this?
Actually I think it's jfc's code; he had a major revamp of alias.c back in the early egcs days and getting it integrated was one of the project's early wins. Sadly, jfc hasn't been involved in GCC work for a long long time.

Anyway, I think the issue here is that once we set new_reg_base_value[regno] to zero, when we do encounter insn 30, we assume that's the first set of (reg 4) and we use the RHS as a base value.

The result is we end up with something in new_reg_base_value[4] that is not correct for the entire life. ie, from function entry to the clobber (reg 4) has a value totally unrelated to what we've stored in new_reg_base_value[4]? RIght?



My memory of all this is fuzzy, but this code in alias.c is effectively trying to compute a set of flow independent base values for each register. If we have two unrelated values going into a register, then we need to essentially say we know nothing about its address and the aliases that creates.








2013-10-24  Eric Botcazou  <ebotcazou@adacore.com>

	PR rtl-optimization/58831
	* alias.c (init_alias_analysis): At the beginning of each iteration, set
  	the reg_seen[N] bit if static_reg_base_value[N] is non-null.


2013-10-24  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.c-torture/execute/pr58831.c: New test.
Seems reasonable to me, assuming my understandings noted above are correct.



jeff


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