[email protected]
[Top] [All Lists]

Re: RFA: fix PR tree-optimization/28144

Subject: Re: RFA: fix PR tree-optimization/28144
From: Roger Sayle
Date: Wed, 5 Jul 2006 08:52:38 -0600 MDT
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;


/* { 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.


<Prev in Thread] Current Thread [Next in Thread>