This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [wide-int] Avoid some changes in output code
- From: Richard Biener <rguenther at suse dot de>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: gcc-patches at gcc dot gnu dot org, zadeck at naturalbridge dot com, mikestump at comcast dot net
- Date: Mon, 4 Nov 2013 10:59:03 +0100 (CET)
- Subject: Re: [wide-int] Avoid some changes in output code
- Authentication-results: sourceware.org; auth=none
- References: <871u30mrv6 dot fsf at talisman dot default> <alpine dot LNX dot 2 dot 00 dot 1311041010361 dot 4261 at zhemvz dot fhfr dot qr> <87habsh661 dot fsf at talisman dot default>
On Mon, 4 Nov 2013, Richard Sandiford wrote:
> Richard Biener <rguenther@suse.de> writes:
> > On Fri, 1 Nov 2013, Richard Sandiford wrote:
> >> I'm building one target for each supported CPU and comparing the wide-int
> >> assembly output of gcc.c-torture, gcc.dg and g++.dg with the corresponding
> >> output from the merge point. This patch removes all the differences I saw
> >> for alpha-linux-gnu in gcc.c-torture.
> >>
> >> Hunk 1: Preserve the current trunk behaviour in which the shift count
> >> is truncated if SHIFT_COUNT_TRUNCATED and not otherwise. This was by
> >> inspection after hunk 5.
> >>
> >> Hunks 2 and 3: Two cases where force_fit_to_type could extend to wider
> >> types and where we weren't extending according to the sign of the source.
> >> We should probably assert that the input is at least as wide as the type...
> >>
> >> Hunk 4: The "&" in:
> >>
> >> if ((TREE_INT_CST_HIGH (arg1) & mask_hi) == mask_hi
> >> && (TREE_INT_CST_LOW (arg1) & mask_lo) == mask_lo)
> >>
> >> had got dropped during the conversion.
> >>
> >> Hunk 5: The old code was:
> >>
> >> if (shift > 0)
> >> {
> >> *mask = r1mask.llshift (shift, TYPE_PRECISION (type));
> >> *val = r1val.llshift (shift, TYPE_PRECISION (type));
> >> }
> >> else if (shift < 0)
> >> {
> >> shift = -shift;
> >> *mask = r1mask.rshift (shift, TYPE_PRECISION (type), !uns);
> >> *val = r1val.rshift (shift, TYPE_PRECISION (type), !uns);
> >> }
> >>
> >> and these precision arguments had two purposes: to control which
> >> bits of the first argument were shifted, and to control the truncation
> >> mask for SHIFT_TRUNCATED. We need to pass a width to the shift functions
> >> for the second.
> >>
> >> (BTW, I'm running the comparisons with CONST_WIDE_INT locally moved to the
> >> end of the !GENERATOR_FILE list in rtl.def, since the current position caused
> >> some spurious differences. The "problem" AFAICT is that hash_rtx hashes
> >> on code, RTL PRE creates registers in the hash order of the associated
> >> expressions, RA uses register numbers as a tie-breaker during ordering,
> >> and so the order of rtx_code can indirectly influence register allocation.
> >> First time I'd realised that could happen, so just thought I'd mention it.
> >> I think we should keep rtl.def in the current (logical) order though.)
> >>
> >> Tested on x86_64-linux-gnu and powerpc64-linux-gnu. OK for wide-int?
> >
> > Bah - can you instead try removing the use of SHIFT_COUNT_TRUNCATED
> > from double-int.c on the trunk? If not then putting this at the
> > callers of wi::rshift and friends is clearly asking for future
> > mistakes.
> >
> > Oh, and I think that honoring SHIFT_COUNT_TRUNCATED anywhere else
> > than in machine instruction patterns was a mistake in the past.
>
> Sure, I can try, but what counts as success? For better or worse:
>
> unsigned int i = 1 << 32;
>
> has traditionally honoured target/SHIFT_COUNT_TRUNCATED behaviour,
> so (e.g.) i is 0 for x86_64 and 1 for MIPS. So if the idea is to
> get rid of SHIFT_COUNT_TRUNCATED at the tree level, it will be a
> visible change (but presumably only for undefined code).
Sure - but the above is undefined (as you say). Generally
constant folding should follow language standards, not machine
dependent behavior.
> > [btw, please use diff -p to get us at least some context if you
> > are not writing changelogs ...]
>
> Yeah, sorry, this whole "no changelog" thing has messed up my workflow :-)
> The scripts I normally use did that for me.
;)
Richard.