RFA: fix PR tree-optimization/28144

[Roger Sayle]

Hi Joern,

On Mon, 26 Jun 2006, Joern RENNECKE wrote:
> As well as fixing the java language issue, this patch also addresses the
> gratuitous inconsistency between -O0 and -O1 for C/C++ (PR27394).

One good reason why overflow in float->int conversion is undefined
in the C/C++ standards (and therefore why PR tree-optimization/27394
was closed as invalid), is because different (current) FP hardware
implements different semantics.  The code not only produces different
results between -O0 and -O1, but between x86 and PowerPC (if memory
serves).  For example, all overflowing FIX on x87 produce the result
-1 (regardless of the original low order bits)!

However, your patch is reasonable not for any "least surprise" for
novice C/C++ programmer's that don't understand FP portability issues
but because GCC's compile-time interpretation attempts to follow the
Java semantics in this respect, where the C/C++ standards allow us a
choice of implementations.


However, your current patch as posted does have its warts...

+	real_from_integer (&l, VOIDmode, (HOST_WIDE_INT) 0,
+			       (HOST_WIDE_INT) -1 << 31, 0);

For the lower bound, the high word should be -1, so that this value
will be interpreted as a negative number, returning a negative real.

+	    real_from_integer (&u, VOIDmode, (HOST_WIDE_INT) 0,
+			       (HOST_WIDE_INT) 32767 << 16 | 65535L, 0);

And this integer constant should never be written as "32767 << 16 |
65535L".  Please be consistent, using either matching shifts, such
as "-1 << 31" and "(1 << 31) - 1", or (when applicable) hexadecimal
constants.

In this case the magic numbers "31" and "32" are reasonable, over the
use of BITS_PER_WORD, to correctly match the Java semantics.

However, (i) you probably also need to consider conversion to unsigned
types, where the lower bound is zero, and the upper is UINT_MAX, and
(ii) the setting of high and low to ut/lt's values when we do overflow
is probably incorrect in these cases, and (iii) you need some additional
testcases to confirm that GCC correctly implements the intended
(Java-like) semantics and maybe even catch logic errors such as those
discussed above.


I think you probably want to write this patch something more like:

	if (!overflow)
	  {
	    HOST_WIDE_INT lhi;
	    unsigned HOST_WIDE_INT llo;
	    REAL_VALUE_TYPE l;

	    if (TYPE_PRECISION (type) >= 32)
	      {
		tree lt = TYPE_MIN_VALUE (type);
		lhi = TREE_INT_CST_HIGH (lt);
		llo = TREE_INT_CST_LOW (lt);
	      }
	    else if (TYPE_UNSIGNED (type))
	      {
		lhi = 0;
		llo = 0;
	      }
	    else
	      {
		lhi = -1;
		llo = (HOST_WIDE_INT) -1 << 31;
	      }

	    real_from_integer (&l, VOIDmode, lhi, llo,
			       TYPE_UNSIGNED (type));

	    if (REAL_VALUES_LESS (r, l))
	      {
		overflow = 1;
		high = lhi;
		low = llo;
	      }

and something similar for the symmetric upper bound case, with uhi
and ulo (slightly complicated by the need to test for TYPE_MAX_VALUE
(type) != NULL_TREE).

Thanks for tackling this.  Please could you post an updated patch
after bootstrap and regression testing, together with a new dejagnu
testcase or two.  Thanks in advance.

Roger
--