Re: [wide-int] Avoid some changes in output code

[Richard Biener <rguenther at suse dot de>]

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.