Re: RFA: fix PR tree-optimization/28144

Re: RFA: fix PR tree-optimization/28144

[Roger Sayle]

Hi Joern,

On Tue, 4 Jul 2006, Joern RENNECKE wrote:
> >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.
>
> The expresions I used are consistent in that they are - AFAIKS - the
> simplest way to express the constants in question.
>
> "(1 << 31) - 1" invokes undefined behaviour.

The preferred "((unsigned HOST_WIDE_INT) 1 << 31) - 1", or even just the
less ideal "(1UL << 31) - 1", doesn't invoke undefined behaviour and is
the correct idiom used elsewhere in GCC.  I apologise if you found my
omission of casts (for clarity) confusing.



> >(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'm pretty sure that we already have testcases for the conversions that
> are defined by the C standard.  I.e. new testcases required would be
> only for conversions that don't have a result defined by the C standard.
> If any target defines expanders for these conversions that produce
> different results, we'd have to mark these tests as unsupported for
> these targets, or not run them for these targets.

Now you're showing your age.  :-) Since 4.0, GCC has tree-ssa optimizers
and testsuite infrastructure to scan tree dumps that can be used to
check the results of tree-level optimizations without worrying about
the target's assembly language and/or RTL expanders.

Something like:

/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-original" } */

unsigned short foo()
{
  return (unsigned short)65536.0;
}

and...

/* { dg-final { scan-tree-dump-times "return 65535" 1 "original" } } */
/* { dg-final { cleanup-tree-dump "original" } } */


/* { dg-do compile } */
/* { dg-options "-O2 -fdump-tree-original" } */

unsigned short foo()
{
  return (unsigned short)(int)65536.0;
}

/* { dg-final { scan-tree-dump-times "return 0" 1 "original" } } */
/* { dg-final { cleanup-tree-dump "original" } } */



I believe this issue can/should probably be fixed in the Java front-end
by construcing tree's that cast floating point types to integer types
narrower than an int, by first casting to "int".  As you hint yourself,
this will also avoids problems on target hardware that may choose to
use different "undefined" behaviour to implement ufixdfqi2.  I'm fairly
sure that the middle-end won't rearrange (unsigned char)(int)(double)x.

This invokes invokes less undefined behaviour in C and GCC's middle-end.
"(byte)300.0" is undefined in C and may be implemented in different ways
on different hardware, but java's intended "(byte)(int)300.0" is
well-defined and portable.

By not fixing this is in the Java front-end, the code you're changing
only addresses the problem of compile-time conversions, leaving the
deeper issue of run-time conversion unresolved/broken.


This would also prevent Java's distintion between types less than
32-bits from those greater than 32-bits from leaking into the middle-end,
which may be counter intuitive semantics on 64-bit or 16-bit targets
in different front-ends.

I'm not strongly opposed to refining the middle-end's FIX_TRUNC_EXPR
semantics, but as shown, tweaking fold-const.c is trickier and riskier
than fixing the more fundamental problem in the java front-end.

Roger
--

Re: RFA: fix PR tree-optimization/28144

[Joern RENNECKE]

Roger Sayle wrote:

>  
>
>The preferred "((unsigned HOST_WIDE_INT) 1 << 31) - 1", or even just the
>less ideal "(1UL << 31) - 1", doesn't invoke undefined behaviour and is
>the correct idiom used elsewhere in GCC.
>
Yes, that makes sense.

(FWIW, the reason why the incorrect high word didn't cause test failures 
in the orgininal
 patch was because I had assumed big endian argument passing, while 
real_from_integer
 actually has the lowpart argument first.  Which also goes to show that 
the interesting cases
 are not values in excess of the int range, but ones that fint into 
ints, but not (unsigned)
char / short.   

>Now you're showing your age.  :-) Since 4.0, GCC has tree-ssa optimizers
>and testsuite infrastructure to scan tree dumps that can be used to
>check the results of tree-level optimizations without worrying about
>the target's assembly language and/or RTL expanders.
>  
>
I'll look into this once we get back into bootstrapland...

>I believe this issue can/should probably be fixed in the Java front-end
>by construcing tree's that cast floating point types to integer types
>narrower than an int, by first casting to "int".  As you hint yourself,
>this will also avoids problems on target hardware that may choose to
>use different "undefined" behaviour to implement ufixdfqi2.
>
The common case is that there is no specific fix{uns}_trunc[sd]f[qh]i2 
expander,
and the fix[sd]fsi2 expander is used, followed by a narrowing conversion;
the latter is a no-op if TRULY_NOOP_CONVERSION allows this, otherwise it
is generated with a truncsi[qh]i2 expander.


> 
>"(byte)300.0" is undefined in C and may be implemented in different ways
>on different hardware, 
>
But it practice you get 44 most or all of the time when doing it on 
hardware.
Giving a different result when optimizing is unhelpful.

>but java's intended "(byte)(int)300.0" is
>well-defined and portable.
>
>By not fixing this is in the Java front-end, the code you're changing
>only addresses the problem of compile-time conversions, leaving the
>deeper issue of run-time conversion unresolved/broken.
>  
>
No, as stated above, the run-time conversion usually behaves just like 
java for
numbers that can be represented as ints.
Before the saturating code was added to fold-const.c, this was also true 
for the
compile-time conversions, thus allowing people to debug their programs 
at -O0.
Now people have to debug their programs with optimizations, which can get
really ugly when the constant folding is only triggered at -O3.

>This would also prevent Java's distintion between types less than
>32-bits from those greater than 32-bits from leaking into the middle-end,
>which may be counter intuitive semantics on 64-bit or 16-bit targets
>in different front-ends.
>  
>
What we currently have is a buggy java conversion code having leaked 
into the
middle-end, making the results counter intuitive on all targets.
It would certainly be nice if we could get this consistent for all 
targets, but this
would require a language callback.  If we want to prevent global language
selection, that would mean that either a callback, the intermediate 
conversion type,
its size, or the context langugae would have embedded into the node 
which causes
the conversion.
I suppose encoding the language in the node is best, because this allows 
to use
a relatively small bitfield, and this can the be used as an index into 
an array
of structures describing the frontend languages, so this would also be a 
very
general mechanism which can be applied to all nodes that have some
language-specific behaviour (this is not a property of the function because
in the future, different conversions could be inlined from functions 
written in
different languages).

>I'm not strongly opposed to refining the middle-end's FIX_TRUNC_EXPR
>semantics, but as shown, tweaking fold-const.c is trickier and riskier
>than fixing the more fundamental problem in the java front-end.
>  
>
If you want to fix it in the java front end, then we should back out the 
java saturation
code from fold-const.c.  This would also get rid of the regression we 
have with
usability of our compiler for debugging C code.

Re: RFA: fix PR tree-optimization/28144

[Tom Tromey]

>>>>> "Roger" == Roger Sayle <roger@eyesopen.com> writes:

Roger> I believe this issue can/should probably be fixed in the Java front-end
Roger> by construcing tree's that cast floating point types to integer types
Roger> narrower than an int, by first casting to "int".

parse.y:patch_cast looks like it tries to do this.
If it is failing in some case, I'd be mildly curious to know why.

I say "mildly" because with the eventual ecj merge, all this code is
going away... java bytecode explicitly represents the intermediate
cast.

Tom

Re: RFA: fix PR tree-optimization/28144

[Joern RENNECKE]

Tom Tromey wrote:

>>>>>>"Roger" == Roger Sayle <roger@eyesopen.com> writes:
>>>>>>            
>>>>>>
>
>Roger> I believe this issue can/should probably be fixed in the Java front-end
>Roger> by construcing tree's that cast floating point types to integer types
>Roger> narrower than an int, by first casting to "int".
>
>parse.y:patch_cast looks like it tries to do this.
>If it is failing in some case, I'd be mildly curious to know why.
>  
>
I haven't actually tested any java code.
My starting point was that I wanted to fix the C inconsistency of 
different results
for different optimization options (it depends on the test case at which 
optimization
level the constant folding is triggered, but for lots of cases -O0 and 
-O3 are different).
The comment in fold-const.c said that the implemented semantics were 
Java semantics.
Since having different behaviour for -O0 and -O3 doesn't make much sense 
with the
java philosophy, I concluded that the run-time java semantics were the 
same, and since
the C semantics are demonstrably different from the fold-const.c 
semantics, I expected
to find some language-dependent code in the expanders.  Placing the same
language-dependence into the constant folding should solve the problem.  
Only, there
isn't any.
Thus, it followed that either fold-const.c or the expanders for the 
runtime code had to
be doing the wrong thing for java, so I went looking what they should do.

If the java frontend makese sure that some conversions are never seen by 
the constant
folder, but if the constant folder seems them, it does things that are 
incorrect for java,
then certainly the comment is wrong to say that this behaviour is 
required by java.