RFA: fix PR tree-optimization/28144/27394

[Roger Sayle]

On Mon, 17 Jul 2006, Joern RENNECKE wrote:
> 2006-07-16  J"orn Rennecke <joern.rennecke@st.com>
>
>	PR tree-optimization/28144/27394
>	* rtl.h (expand_fix_1): Declare.
>	* optabs.c (expand_fix_1): New function, broken out of:
>	(expand_fix).
>	* fold-const.c (fold_convert_const_int_from_real): Fix comment
>	wrt. implementing Java conversions.
>	Perform conversion in same mode as optabs.c would do it.
>
>	gcc.c-torture/execute/pr27394.c: New test.

Hmm.... I guess this is OK for mainline once we return to stage1
with the minor corrections described at the end of this review.


I'll grant you that although PR 27394 was closed as invalid, and PR 28144
is equally invalid (save for the ambiguous wording of the comment in
fold-const.c), producing the same results for expressions evaluated at
compile-time and run-time is a valid quality-of-implementation issue.

However, I'll point out that your patch still doesn't guarantee that
we'll produce the same results for floating point truncation at all
optimization levels.  Consider the ouput of the following program on
x86 at -O2 and -O3.

#include <stdio.h>

void foo(double x)
{
  int y = (int)x;
  printf("%d\n",y);
}

int main()
{
  foo(10e10);
  return 0;
}


Even with your patch, I get "-2147483648" at -O2 (run-time evaluation)
and "2147483647" at -O3 (compile-time evaluation).  As explained in
rtl.texi for both fix and unsigned_fix: "how rounding is done is not
specified".  So although you jump through the hoops to find out what
mode the truncation will be done in, the behaviour on overflow in that
mode is still just as undefined.  This is precisely the freedom that
allows optabs.c to use a wider integer mode for truncations!

That you happen to be debugging a invalid testcase on a target where
complicating the middle-end a little may fortunately generate the correct
result, is only working around and/or reducing the frequency of this
"feature".  There's a good reason language standards call this undefined.

I'm also beginning to suspect it might be useful to have a diagnostic,
"undefined conversion invoked at compile-time, run-time results may
differ", or something similar.  Or perhaps even a new option to
prohibit the middle-end from taking advantage of the undefinedness
of FP->int conversion overflow, forcing all such operations to be
performed at run-time.


But I'll concede that targets such as SH that don't provide a
fixdfqi2, cause the middle-end to issue a fixdfsi2, which may be
wide-enough to avoid undefined backend behaviour for suitable
argument values, and your fix allows us to get the same results at
compile-time as run-time in these cases.


Corrections:

Firstly, in the ChangeLog entry you should write one PR number
per line.

	PR tree-optimization/28144
	PR tree-optimization/27394

not

	PR tree-optimization/28144/27394

+/* As above, but don't actually emit any code if FROM and TO are NULL_RTX.
+   Return the mode that is actuallyu the target of the floating
+   point -> integer conversion.  Reset *UNSIGNEDPP if we use a widened
+   signed conversion to implement an unsigned conversion.  */

Typo: "actuallyu" -> "actually".


If anyone else has comments or opinions about Joern's patch, please feel
free to post them.  I'm still not 100% sure how I feel about it myself.

Roger
--