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]

[RFA:] Use of find_reg_note in FIND_REG_INC_NOTE causes dbr failure


First, speaking of dbr-related failures, here's an unreviewed patch:
<URL:http://gcc.gnu.org/ml/gcc-patches/2001-08/msg00575.html> (Patch for
bug in reorg.c: fill_slots_from_thread missing update of own_thread).
Thanks in advance for reviewing that and this one.

Now to the one at hand: for CRIS, gcc.c-torture/execute/memcpy-bi.c,
string-opt-12.c and string-opt-5.c fail without this patch because there's
wrong code after the dbr pass (at least for memcpy-bi.c): the wrong
register is substituted for one of the moves from src.  But for once, it
doesn't seem to be a bug in reorg.c.

The function reg_set_p uses REG_INC_NOTE which checks pointer equality of
the note.  The call comes from reorg.c:fill_slots_from_thread, where
incidentally pointer equality for registers is not guaranteed;
regrename.c:do_replace can create duplicate REG rtx:es.  That's not the
only place where pseudos can be mapped to the same hard reg, but here we
have the *same* pseudo in different rtx:es, so worth mentioning.  The code
in reorg.c:fill_slots_from_thread where the bad substitution happens is:

      if (GET_CODE (trial) == INSN && GET_CODE (pat) == SET
	  && GET_CODE (SET_SRC (pat)) == REG
	  && GET_CODE (SET_DEST (pat)) == REG)
	{
	  rtx next = next_nonnote_insn (trial);

	  if (next && GET_CODE (next) == INSN
	      && GET_CODE (PATTERN (next)) != USE
	      && ! reg_set_p (SET_DEST (pat), next)
	      && ! reg_set_p (SET_SRC (pat), next)
	      && reg_referenced_p (SET_DEST (pat), PATTERN (next))
	      && ! modified_in_p (SET_DEST (pat), next))
	    validate_replace_rtx (SET_DEST (pat), SET_SRC (pat), next);
	}

The insn "trial" (container of "pat", the BODY of "trial") is:

 (insn 433 146 148 (set (reg:SI 2 r2 [57])
        (reg/f:SI 4 r4 [105])) 32 {*movsi_internal} (insn_list 146 (insn_list 146 (nil)))
     (nil))

"next" is:

 (insn 477 148 149 (set (reg:SI 13 r13)
         (mem/s:SI (post_inc:SI (reg:SI 2 r2 [57])) 0)) 32 {*movsi_internal} (insn_list 433 (nil))
     (expr_list:REG_INC (reg:SI 2 r2 [57])
         (nil)))

The (reg:SI 2 r2 [57]) in the REG_INC note in "next" and the one in
"trial" are different objects (pointerwise), as allocated by
regrename.c:do_replace.  The first "&& ! reg_set_p (SET_DEST (pat), next)"
should have caught this, but reg_set_p uses FIND_REG_INC_NOTE (insn, reg)
to guard against the presence of postincrements such as this one.  But
FIND_REG_INC_NOTE is for autoinc machines defined to "(find_reg_note
((insn), REG_INC, (reg)))", and the function find_reg_note compares
pointers when checking "reg" for a match.  So it fails to match the
duplicated REG it is passed.  Any required change seems to be limited to
the definition of FIND_REG_INC_NOTE, its callers and the function
find_reg_note.  I've taken one of the possible routes.

There's a neighboring function, find_regno_note, that seems better for the
case at hand, since it tests register-number equality, and also overlap
(for hard registers).  The drawback is that it wants the REG_INC note to
actually be for a *register*, and does not find other REG_INCs.

Many current callers of FIND_REG_INC_NOTE pass NULL_RTX for the note to
match, and could just as well be using side_effects_p as a change "Fri Aug
6 16:53:55 EDT 1999 John Wehle (john@feith.com)" in ChangeLog.2 indicates,
if it weren't for that for non-autoinc machines FIND_REG_INC_NOTE is
defined to 0.  Most of the rest of the callers test elsewhere that the
REG_INCs they pass, or the non-zero return-value, is a REG, and so would
be better off with FIND_REG_INC_NOTE using find_regno_note.  The exception
is a use in reload.c, but it seems to be a case not normally encountered
(an autoincrement of a MEM that needs to be replaced) so it could just as
well use find_reg_note directly.  But changing FIND_REG_INC_NOTE to call
find_regno_note for REGs seems best.  The (rtx) casts are necessary
because some callers pass 0, not NULL_RTX.

Built and checked with no regressions on h8300-hms+h8300-sim (with
--enable-languages=objc), mn10300-elf+mn10300-sim and
powerpc-eabisim+powerpc-sim.  The other autoinc simulator targets don't
build with sources as of "Wed Sep 12 09:04:36 GMT 2001".  Note that
i686-pc-linux-gnu is not an autoinc machine and isn't affected by this
change.  In order to get the patch tested on more autoinc machines, I
applied kaz Kojima's patch at
<URL:http://gcc.gnu.org/ml/gcc-patches/2001-09/msg00009.html> and built
and tested on sh-elf+sh-hms-sim too (with that patch applied only for that
target), with no regressions.

Ok to commit?

	* rtl.h (FIND_REG_INC_NOTE) [HAVE_PRE_INCREMENT
	|| HAVE_PRE_DECREMENT || HAVE_POST_INCREMENT
	|| HAVE_POST_DECREMENT]: Call find_regno_note for REGs.

Index: rtl.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/rtl.h,v
retrieving revision 1.294
diff -p -c -r1.294 rtl.h
*** rtl.h	2001/08/30 20:44:50	1.294
--- rtl.h	2001/09/12 03:47:22
*************** extern unsigned int subreg_regno 	PARAMS
*** 944,950 ****
  
  /* Don't continue this line--convex cc version 4.1 would lose.  */
  #if (defined (HAVE_PRE_INCREMENT) || defined (HAVE_PRE_DECREMENT) || defined (HAVE_POST_INCREMENT) || defined (HAVE_POST_DECREMENT))
! #define FIND_REG_INC_NOTE(insn, reg) (find_reg_note ((insn), REG_INC, (reg)))
  #else
  #define FIND_REG_INC_NOTE(insn, reg) 0
  #endif
--- 944,953 ----
  
  /* Don't continue this line--convex cc version 4.1 would lose.  */
  #if (defined (HAVE_PRE_INCREMENT) || defined (HAVE_PRE_DECREMENT) || defined (HAVE_POST_INCREMENT) || defined (HAVE_POST_DECREMENT))
! #define FIND_REG_INC_NOTE(insn, reg)				\
!   (reg != NULL_RTX && REG_P ((rtx) (reg))			\
!    ? find_regno_note ((insn), REG_INC, REGNO ((rtx) (reg)))	\
!    : find_reg_note ((insn), REG_INC, (reg)))
  #else
  #define FIND_REG_INC_NOTE(insn, reg) 0
  #endif

brgds, H-P


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