| Message ID | 20101206120402.jqbfggfnk0gk4kgs-nzlynne@webmail.spamcop.net |
|---|---|
| State | New |
| Headers | show |
> PR target/46739 > * config/mmix/mmix.c (mmix_intval): Split shift count. Ok. r~
On Mon, 6 Dec 2010, Joern Rennecke wrote:
> Cross-built on x86_64-pc-linux-gnu.
(not quotable from within pine, but no specific need this time)
Ok, thanks! Feel free to CC me on CRIS and MMIX issues; I'm
frequently behind on my mailing list reading.
brgds, H-P
I don't think the code is correct either before or after this patch;
aren't the halves of a CONST_DOUBLE in units of HOST_WIDE_INT not long?
The whole block
/* We make a little song and dance because converting to long long in
gcc-2.7.2 is broken. I still want people to be able to use it for
cross-compilation to MMIX. */
if (GET_CODE (x) == CONST_DOUBLE && GET_MODE (x) == VOIDmode)
{
if (sizeof (HOST_WIDE_INT) < sizeof (HOST_WIDEST_INT))
{
retval = (unsigned) CONST_DOUBLE_LOW (x) / 2;
retval *= 2;
retval |= CONST_DOUBLE_LOW (x) & 1;
retval |=
(unsigned HOST_WIDEST_INT) CONST_DOUBLE_HIGH (x)
<< (HOST_BITS_PER_LONG);
}
else
retval = CONST_DOUBLE_HIGH (x);
return retval;
}
should be substantially simplified given that 2.95 not 2.7.2 is the
minimum version for building a cross compiler. (Maybe double-int.[ch]
should have functions to convert a double_int value to either signed or
unsigned HOST_WIDEST_INT.)
On 12/06/2010 09:21 AM, Joseph S. Myers wrote: > I don't think the code is correct either before or after this patch; > aren't the halves of a CONST_DOUBLE in units of HOST_WIDE_INT not long? Yes, they are. > (Maybe double-int.[ch] > should have functions to convert a double_int value to either signed or > unsigned HOST_WIDEST_INT.) Sure. double_int_to_{s,u}widest{,_p}? r~
Quoting "Joseph S. Myers" <joseph@codesourcery.com>: > I don't think the code is correct either before or after this patch; > aren't the halves of a CONST_DOUBLE in units of HOST_WIDE_INT not long? That is true; but this is really a separate issue. Note, for this to break, there have to be two distinct type sizes larger than that of long. Also worrying is the cast of the low part to unsigned, that will even break if we have int / long / long long (or int128) as 32 / 64 / 128 bit. > The whole block ... > should be substantially simplified given that 2.95 not 2.7.2 is the > minimum version for building a cross compiler. It looks as if this was a specific decision to support 2.7.2 by the port maintainer, albeit that was a while ago. I'll leave it to the port maintainers to decide if this is now obsolete.
On Mon, 6 Dec 2010, Joern Rennecke wrote: > Quoting "Joseph S. Myers" <joseph@codesourcery.com>: > > I don't think the code is correct either before or after this patch; > > aren't the halves of a CONST_DOUBLE in units of HOST_WIDE_INT not long? > > That is true; but this is really a separate issue. Note, for this to > break, there have to be two distinct type sizes larger than that of long. > Also worrying is the cast of the low part to unsigned, that will even break > if we have int / long / long long (or int128) as 32 / 64 / 128 bit. > > > The whole block > ... > > should be substantially simplified given that 2.95 not 2.7.2 is the > > minimum version for building a cross compiler. > > It looks as if this was a specific decision to support 2.7.2 by the port > maintainer, albeit that was a while ago. I'll leave it to the port > maintainers to decide if this is now obsolete. ...well, quite... :-) (No, I didn't consider the context of the patch and as you say, it's a separate issue. Thanks everyone for caring.) brgds, H-P
On Mon, 6 Dec 2010, Joern Rennecke wrote: > > should be substantially simplified given that 2.95 not 2.7.2 is the > > minimum version for building a cross compiler. > > It looks as if this was a specific decision to support 2.7.2 by the port > maintainer, albeit that was a while ago. I'll leave it to the port > maintainers to decide if this is now obsolete. It's obviously obsolete, since ports can't unilaterally support older host compilers than the rest of GCC.
Quoting "Joseph S. Myers" <joseph@codesourcery.com>: > On Mon, 6 Dec 2010, Joern Rennecke wrote: > >> > should be substantially simplified given that 2.95 not 2.7.2 is the >> > minimum version for building a cross compiler. >> >> It looks as if this was a specific decision to support 2.7.2 by the port >> maintainer, albeit that was a while ago. I'll leave it to the port >> maintainers to decide if this is now obsolete. > > It's obviously obsolete, since ports can't unilaterally support older host > compilers than the rest of GCC. It's not quite that obvious. When we say we require a minimum version of GCC, we may say this because of same part that is not built for all targets, or for which a miscompilation might be a non-issue for some target as the code is never reached. Or some issues might be host-specific. Some issues might be sporadic, triggered by particular code patterns, and very time consuming to specify what host/target combinations are affected. The hack in mmix_intval looks like it works around an issue in this category.
Index: config/mmix/mmix.c =================================================================== --- config/mmix/mmix.c (revision 167494) +++ config/mmix/mmix.c (working copy) @@ -2747,7 +2747,7 @@ mmix_intval (rtx x) retval |= (unsigned HOST_WIDEST_INT) CONST_DOUBLE_HIGH (x) - << (HOST_BITS_PER_LONG); + << (HOST_BITS_PER_LONG)/2 << (HOST_BITS_PER_LONG)/2; } else retval = CONST_DOUBLE_HIGH (x);