This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: fix avoid random results from do_add
- From: Joern Rennecke <joern dot rennecke at superh dot com>
- To: rth at redhat dot com (Richard Henderson)
- Cc: joern dot rennecke at superh dot com (Joern Rennecke), gcc-patches at gcc dot gnu dot org
- Date: Mon, 7 Jun 2004 21:14:40 +0100 (BST)
- Subject: Re: RFA: fix avoid random results from do_add
> And can you verify that using real_hash fixes the problem? That's
> supposed to ignore all fields that are not in use, and I believe the
> proper way to solve this problem. Otherwise you'll iteratively find
> many many other places where not all fields are set or cleared.
Yes, it fixes the problem. Or at least the build suceeds and the compiler
can be tested. The problem is now only reproducible when running a
complete build from a cron script, so I can't find out if there was
actually any hashing of constants differing in the signalling/canonical
fields going on, or if by chance we got consistent values to start with.
I did look at the other arithmetic functions around do_add initially, and
found that they initialize the result with get_zero, which is why I had
assumed that we are supposed to have results that are consistent in their
binary implementation. OTOH when you asked about the place where the
supposedly unused fields matter, I checked for other users of
TREE_REAL_CST_PTR to see if the problem is more widespread, and came up
with a use in varasm.c - but that is safe because it uses real_hash.
Using real_hash certainly looks cleaner. There is some performance penalty
to pay, but I don't think it should matter, floating point constants are
not that prevalent.
I was wondering if I should xor the result of real_hash into val, or use
iterative_hash_object to combine it into val, but that would take advantage
of hashval_t being the same width or wider and assignment compatible to the
return type of real_hash; I suppose that might change in the future.
I have bootstrapped the patch below also on i686-pc-linux-gnu,
and so far the regression check has done the entire C testsuite without
any new regressions, and it's now chugging along on g++.dg/dg.exp .
2004-06-07 J"orn Rennecke <joern.rennecke@superh.com>
* tree.c (iterative_hash_expr): Use real_hash.
Index: tree.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree.c,v
retrieving revision 1.373
diff -p -r1.373 tree.c
*** tree.c 21 May 2004 00:54:35 -0000 1.373
--- tree.c 7 Jun 2004 19:47:19 -0000
*************** iterative_hash_expr (tree t, hashval_t v
*** 3846,3853 ****
val = iterative_hash_object (TREE_INT_CST_HIGH (t), val);
}
else if (code == REAL_CST)
! val = iterative_hash (TREE_REAL_CST_PTR (t),
! sizeof (REAL_VALUE_TYPE), val);
else if (code == STRING_CST)
val = iterative_hash (TREE_STRING_POINTER (t),
TREE_STRING_LENGTH (t), val);
--- 3846,3856 ----
val = iterative_hash_object (TREE_INT_CST_HIGH (t), val);
}
else if (code == REAL_CST)
! {
! unsigned int val2 = real_hash (TREE_REAL_CST_PTR (t));
!
! val = iterative_hash (&val2, sizeof (unsigned int), val);
! }
else if (code == STRING_CST)
val = iterative_hash (TREE_STRING_POINTER (t),
TREE_STRING_LENGTH (t), val);