Re: RFA: fix PR tree-optimization/28144/27394

[Joern RENNECKE <joern dot rennecke at st dot com>]

Roger Sayle wrote:

> 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!
>  
Yes, if there is still an overflow in the mode we are doing the actual 
conversion in,
the run-time conversion results are target dependent.  We would require 
a target hook
in order to generate the same compile-time results then.
However, just because optimization-level independent debugging is hard 
to archive in
general, does not mean we should not allow it to work when the effort to 
archive it is
relatively small.

> 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.
>  
Well, SImode is guaranteed to be at least 32 bits wide, so the testcase 
is known to
work for all targets that don't define an fixdfqi pattern (if qimode is 
16 bits, we
have already won)  And the only current gcc target with and fixtruncdfqi 
pattern
is m68k, and this pattern is just like the fixtruncdfhi pattern, except 
that it truncates
QImode rather than HImode.
So that the testcase works is not just luck, it works on all current 
targets, and it is
unlikely that we ever get a target where we would have to mark this as 
an unsupported
test.

> 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.
Yes, that would be useful.  However, in order to do that, the tree node that
causes the conversion to happen would have to encode enough information so
that we know what the language semantics say about this conversion.

I wonder how the target-dependence affects Java.  Does compiling to 
native code
actually introduce all the target dependencies of the conversions into 
the Java
run-time behaviour - thus violating the standard?  Or does the java 
front end do anything
to counteract this problem?  But if it does, it would seem funny that we 
have to worry about
folding things right for Java in fold-const.c

>  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.
>  
This would be of rather limited value, since it would reduce the amount of
optimizations we do.  Say you have a program that only works because tree
optimization folds two expressions consistently at compile time.  You make a
minor change of the code which shifts the conversion of one of the 
expressions
to run-time execution, causing an internal inconsistency.  You use your now
evaluate-at-runtime option, and because both expressions are evaluated 
at run time,
the program is consistent again.  This still doesn't tell you where the 
bug in the
program is.  Note, this option masking a bug is not conclusive; e.g. it 
can prevent
an inlining which used to expose a latent alias bug in the program ... 
or in the compiler.


> 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.
>  
As stated above, this is not the exception, but the rule for values that 
fit in SImode.

> 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".
>  
Thanks.