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]

[Committed] PR middle-end/4520 causes PR bootstrap/28784


I was finally frustrated enough with the bootstrap comparison failure
of java/decl.o when building on ia64-unknown-linux-gnu to go hunting
for its cause.  After two and a half solid days of boostrapping with
diagnostic output I finally managed confirm that the underlying cause
is PR middle-end/4520; a very old enhancement request submitted by
David pointing out the cselib shouldn't hash on the value of a pointer
but instead on the pointer's contents.

The precise chain of events is long and convoluted but essentially a
difference in memory allocations (affected by system load, garbage
collection, compiler options, debuggging statements) results in us
producing a different hash in cselib, which results in us returning
a different result from cselib_subst_to_values, which results in a
different canonical RTX in canon_rtx, which generates a different
result in base_alias_check, which generates a different result in
true_dependence, which creates a different set of LOG_LINKS on an
instruction, which creates a different INSN_DEPEND chain for the
scheduler, which returns a different value for priority when selecting the
next insn to schedule, which naturally affects rank_for_schedule, and
ultimately the order of instructions produced after the ia64's second
instruction scheduling pass.

The audit trail in PR bootstrap/28784 mentions PR20586 and PR28752,
basically a history of intermitent bootstrap comparison failures on
ia64 that mysterious come and go.  Given the cause of this failure,
and how slippery it is (I managed to loose it several times with
completely unrelated changes in the code), I suspect they may all
be fallout from PR4520.  Alas PR4520 is only an enhancement request,
and therefore not of itself suitable for stage2 or stage3.  I wonder
if the GCC cycle time were shorter whether this would have been fixed
before now.


The solution is simple.  cselib.c's cselib_hash_rtx is a specialized
version of (and derived from) cse.c's hash_rtx.  It appears that the
potential comparison failures in the LABEL_REF and SYMBOL_REF cases
of hash_rtx were identified and corrected long ago, but those changes
were never applied to the near identical code in cselib_hash_rtx.
Ahh, the perils of code duplication.  Perhaps one day I'll investigate
why hash_rtx and cselib_hash_rtx are different, and attempt to factor
the common functionality.  But for stage3, fixing the problem seems
sufficient.

The following patch has been tested on ia64-unknown-linux-gnu together
with the recently approved/committed patch for PR target/28672, with a
full "make bootstrap", all default languages, which no finishes and
regression tested with a top-level "make -k check", where the results
look similar to my last results on the same machine from June.

Just because I'm paranoid and we're also in stage3, I've also tested
this patch on i686-pc-linux-gnu with a full bootstrap and regression
test, all languages including Ada, with no new failures, to confirm
there are no unexpected surprises there.

Committed to mainline as revision 116891.  Thanks to DJE for filing
PR4520 and my apologies for not addressing the issue sooner.


2006-09-12  Roger Sayle  <roger@eyesopen.com>

	PR middle-end/4520
	PR bootstrap/28784
	* cselib.c (cselib_hash_rtx): Avoid hashing on the address of labels
	and symbols.  Instead use the implementation from cse.c's hash_rtx.


Index: cselib.c
===================================================================
*** cselib.c	(revision 116509)
--- cselib.c	(working copy)
*************** cselib_hash_rtx (rtx x, int create)
*** 630,643 ****

        /* Assume there is only one rtx object for any given label.  */
      case LABEL_REF:
!       hash
! 	+= ((unsigned) LABEL_REF << 7) + (unsigned long) XEXP (x, 0);
        return hash ? hash : (unsigned int) LABEL_REF;

      case SYMBOL_REF:
!       hash
! 	+= ((unsigned) SYMBOL_REF << 7) + (unsigned long) XSTR (x, 0);
!       return hash ? hash : (unsigned int) SYMBOL_REF;

      case PRE_DEC:
      case PRE_INC:
--- 630,657 ----

        /* Assume there is only one rtx object for any given label.  */
      case LABEL_REF:
!       /* We don't hash on the address of the CODE_LABEL to avoid bootstrap
! 	 differences and differences between each stage's debugging dumps.  */
!       hash += (((unsigned int) LABEL_REF << 7)
! 	       + CODE_LABEL_NUMBER (XEXP (x, 0)));
        return hash ? hash : (unsigned int) LABEL_REF;

      case SYMBOL_REF:
!       {
! 	/* Don't hash on the symbol's address to avoid bootstrap differences.
! 	   Different hash values may cause expressions to be recorded in
! 	   different orders and thus different registers to be used in the
! 	   final assembler.  This also avoids differences in the dump files
! 	   between various stages.  */
! 	unsigned int h = 0;
! 	const unsigned char *p = (const unsigned char *) XSTR (x, 0);
!
! 	while (*p)
! 	  h += (h << 7) + *p++; /* ??? revisit */
!
! 	hash += ((unsigned int) SYMBOL_REF << 7) + h;
! 	return hash ? hash : (unsigned int) SYMBOL_REF;
!       }

      case PRE_DEC:
      case PRE_INC:


Roger
--


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