[PATCH] Update COND_EXPR section of c-tree.texi

[PATCH] Update COND_EXPR section of c-tree.texi

[Roger Sayle]

My recent patch to fix PR fortran/7388 corrected transformations on
COND_EXPR tree nodes that didn't preserve the noreturn semantics of
operands with void type.  Although this interpretation is described
both in tree.def and some of the routines in fold-const.c, its not
surprising that its not widely known, as it isn't mentioned at all
in the GCC internals documentation on COND_EXPR in c-tree.texi.

The following documentation patch corrects the situation.  It also
reduces the emphasis on GCC's extension to allow "x ? : 3", which
is a front-end feature and described in user documentation, and in
this internals documentation just explains that this syntactic sugar
should be expanded before it reaches GCC's middle-end.

Checked with "make dvi" on i686-pc-cygwin.

Ok for mainline?



2002-10-17  Roger Sayle  <roger@eyesopen.com>

	* doc/c-tree.texi: Update description of COND_EXPR tree nodes.


Index: c-tree.texi
===================================================================
RCS file: /cvs/gcc/gcc/gcc/doc/c-tree.texi,v
retrieving revision 1.35
diff -c -3 -p -r1.35 c-tree.texi
*** c-tree.texi	4 Sep 2002 17:35:58 -0000	1.35
--- c-tree.texi	16 Oct 2002 02:56:36 -0000
*************** These nodes represent @code{?:} expressi
*** 2127,2151 ****
  is of boolean or integral type.  If it evaluates to a nonzero value,
  the second operand should be evaluated, and returned as the value of the
  expression.  Otherwise, the third operand is evaluated, and returned as
! the value of the expression.  As a GNU extension, the middle operand of
! the @code{?:} operator may be omitted in the source, like this:

! @example
! x ? : 3
! @end example
! @noindent
! which is equivalent to

! @example
! x ? x : 3
! @end example
!
! @noindent
! assuming that @code{x} is an expression without side-effects.  However,
! in the case that the first operation causes side effects, the
! side-effects occur only once.  Consumers of the internal representation
! do not need to worry about this oddity; the second operand will be
! always be present in the internal representation.

  @item CALL_EXPR
  These nodes are used to represent calls to functions, including
--- 2127,2147 ----
  is of boolean or integral type.  If it evaluates to a nonzero value,
  the second operand should be evaluated, and returned as the value of the
  expression.  Otherwise, the third operand is evaluated, and returned as
! the value of the expression.

! The second operand must have the same type as the entire expression,
! unless it unconditionally throws an exception or calls a noreturn
! function, in which case it should have void type.  The same constraints
! apply to the third operand.  This allows array bounds checks to be
! represented conveniently as @code{(i >= 0 && i < 10) ? i : abort()}.

! As a GNU extension, the C language front-ends allow the second
! operand of the @code{?:} operator may be omitted in the source.
! For example, @code{x ? : 3} is equivalent to @code{x ? x : 3},
! assuming that @code{x} is an expression without side-effects.
! In the tree representation, however, the second operand is always
! present, possibly protected by @code{SAVE_EXPR} if the first
! argument does cause side-effects.

  @item CALL_EXPR
  These nodes are used to represent calls to functions, including


Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Mark Mitchell]

--On Thursday, October 17, 2002 12:53:04 PM -0600 Roger Sayle 
<roger@eyesopen.com> wrote:

>
> My recent patch to fix PR fortran/7388 corrected transformations on
> COND_EXPR tree nodes that didn't preserve the noreturn semantics of
> operands with void type.  Although this interpretation is described
> both in tree.def and some of the routines in fold-const.c, its not
> surprising that its not widely known, as it isn't mentioned at all
> in the GCC internals documentation on COND_EXPR in c-tree.texi.

OK.

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Richard Henderson]

On Thu, Oct 17, 2002 at 12:53:04PM -0600, Roger Sayle wrote:
> ! The second operand must have the same type as the entire expression,
> ! unless it unconditionally throws an exception or calls a noreturn
> ! function, in which case it should have void type.  The same constraints
> ! apply to the third operand.  This allows array bounds checks to be
> ! represented conveniently as @code{(i >= 0 && i < 10) ? i : abort()}.

Is this really necessary?  As a front-end thing, maybe, but it
would seem that the best way to handle this is to use

	(i >= 0 ? i : (abort(), 0))


r~

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Roger Sayle]

On Fri, 18 Oct 2002, Richard Henderson wrote:
> On Thu, Oct 17, 2002 at 12:53:04PM -0600, Roger Sayle wrote:
> > ! The second operand must have the same type as the entire expression,
> > ! unless it unconditionally throws an exception or calls a noreturn
> > ! function, in which case it should have void type.  The same constraints
> > ! apply to the third operand.  This allows array bounds checks to be
> > ! represented conveniently as @code{(i >= 0 && i < 10) ? i : abort()}.
>
> Is this really necessary?  As a front-end thing, maybe, but it
> would seem that the best way to handle this is to use
>
> 	(i >= 0 ? i : (abort(), 0))


As you appreciate, I didn't come up with these semantics, I just
documented them.  I agree that the construct you describe above
could be used instead, but given there was already code in GCC
to support it, I though it as easy to fix that.

When I was analysing the failure of PR fortran/7388.f, I was unhappy
that the initial RTL-level dump 7388.f.00.rtl generated by the command
g77 -O2 -fbounds-check -dr testsuite/g77.dg/7388.f contains:

(call_insn 42 41 43 (set (reg:SI 0 eax)
        (call (mem:QI (symbol_ref:SI ("s_rnge")) [0 S1 A8])
            (const_int 16 [0x10]))) -1 (nil)
    (expr_list:REG_NORETURN (const_int 0 [0x0])
        (nil))
    (nil))

(barrier 43 42 45)

(insn 45 43 47 (set (reg:SI 65)
        (reg:SI 0 eax)) -1 (nil)
    (nil))

(insn 47 45 49 (set (reg:SI 66)
        (reg:SI 65)) -1 (nil)
    (nil))

(insn 49 47 51 (parallel[
            (set (reg:SI 67)
                (ashift:SI (reg:SI 66)
                    (const_int 2 [0x2])))
            (clobber (reg:CC 17 flags))
        ] ) -1 (nil)
    (nil))

and so on...  Basically immediately following a barrier in RTL, there's
a sequence of seven unreachable instructions before the code path jumps
to a reachable code label.  Then by the time we reach 7388.f.09.addressof
the RTL contains uses of (reg:SI 89) that never gets set anywhere!

My conclusion was that this code following the barrier was the result
of taking something innocent like "(i>=0? i : (abort(), 0))" and then
using this in ptr[i*4], where more and more expressions are being
folded across the "abort()", including SAVE_EXPRs etc...

After discovering the probable cause, I read through the code in
expand_expr for handling COND_EXPR, and came across the tests for
void_type_node (line expr.c:8702 on mainline), and sure enough
the description in tree.def, described the VOID_TYPE_P convention
that would prevent the dubious initial RTL from being generated.

I'm not claiming that there isn't another bug somewhere in GCC's
constant folding that effectively moves critical instructions after
a noreturn function call, but fixing VOID_TYPE_P in COND_EXPRs has
solved this issue.

7388.f is responsible for over half of the unexpected failures
of mainline on i686-pc-linux-gnu.

Roger
--

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Roger Sayle]

I recently wrote:
> My conclusion was that this code following the barrier was the result
> of taking something innocent like "(i>=0? i : (abort(), 0))" and then
> using this in ptr[i*4], where more and more expressions are being
> folded across the "abort()", including SAVE_EXPRs etc...
>
> I'm not claiming that there isn't another bug somewhere in GCC's
> constant folding that effectively moves critical instructions after
> a noreturn function call, but fixing VOID_TYPE_P in COND_EXPRs has
> solved this issue.

Spooky, but one of the contributing factors to this problem may
have been discovered/fixed earlier today by Nathan Sidwell:
http://gcc.gnu.org/ml/gcc-patches/2002-10/msg01120.html

Nathan, this thread is about my patch to fold_binop_with_conditional
in http://gcc.gnu.org/ml/gcc-patches/2002-10/msg00873.html, which
appears to avoid a problem very similar to the one you describe.

Roger
--

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Nathan Sidwell]

Richard Henderson wrote:

> 
> Is this really necessary?  As a front-end thing, maybe, but it
> would seem that the best way to handle this is to use
> 
> 	(i >= 0 ? i : (abort(), 0))
FE's must canonicalize 'cond ?: false' after all ...

nathan

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Roger Sayle]

> Richard Henderson wrote:
> > Is this really necessary?  As a front-end thing, maybe, but it
> > would seem that the best way to handle this is to use
> >
> > 	(i >= 0 ? i : (abort(), 0))
> FE's must canonicalize 'cond ?: false' after all ...

The patch that introduced these semantics (without documentation) was:
http://gcc.gnu.org/ml/gcc-patches/2001-02/msg01075.html

2001-02-17  Mark Mitchell  <mark@codesourcery.com>

        * fold-const.c (fold_binary_op_with_conditional_arg): New
        function, split out from ...
        (fold): ... here.
        * tree.def (COND_EXPR): Document the use of VOID_TYPE for
        conditional arms that throw exceptions.

which resolved PR c++/218.  In all fairness to Mark for adding this
functionality, has anyone actually tested whether Richards suggested
idiom "(i>=0 ? i : (abort(), 0))" currently works?

Note that the current situation in the g77 front-end is that s_rng in
libF77 already has the appropriate return type, and the middle-end is
still managing to corrupt uses of "((i>=0) && (i<10)? i : s_rng())",
when s_rng is marked no return.  Obviously the compound expression alone
isn't going to solve the problem.

Perhaps once Nathan's patch is approved (which should improve things)
we can consider alternate solutions for PR c++/218 and PR fortran/7388,
and revert some of the above patch.

It should also be noted that the VOID_EXPR solution allows GCC to
generate less and more efficient RTL, by eliminating dead code
earlier, and avoiding SAVE_EXPRs where no longer required.  I'll
check to see if we have noreturn predicates that'll permit these
savings with compound (and other) expressions.

(i>=0 ? i : (abort(),0)) + c need not become (i>=0 ? i+c : (abort(),c))
for arbitrarily complex c, and s_rng()+c should be folded to s_rng().
Indeed "(noreturn(),x)" could become "(typeof(x))noreturn()", so in
our example above (int)abort().

I'll see what I can come up with.

Roger
--

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Mark Mitchell]

--On Saturday, October 19, 2002 10:42:46 AM -0600 Roger Sayle 
<roger@eyesopen.com> wrote:

>
>> Richard Henderson wrote:
>> > Is this really necessary?  As a front-end thing, maybe, but it
>> > would seem that the best way to handle this is to use
>> >
>> > 	(i >= 0 ? i : (abort(), 0))
>> FE's must canonicalize 'cond ?: false' after all ...
>
> The patch that introduced these semantics (without documentation) was:
> http://gcc.gnu.org/ml/gcc-patches/2001-02/msg01075.html
>
> 2001-02-17  Mark Mitchell  <mark@codesourcery.com>
>
>         * fold-const.c (fold_binary_op_with_conditional_arg): New
>         function, split out from ...
>         (fold): ... here.
>         * tree.def (COND_EXPR): Document the use of VOID_TYPE for
>         conditional arms that throw exceptions.
>
> which resolved PR c++/218.  In all fairness to Mark for adding this
> functionality, has anyone actually tested whether Richards suggested
> idiom "(i>=0 ? i : (abort(), 0))" currently works?

I'm pretty sure that I wasn't really adding semantics with that patch;
I was probably just making the way things always were inch a little
closer to working.

Note that the C++ standard says that "throw 1" has type "void", and
that it is OK to have a conditional expression like:

  b ? throw 1 : x

See [expr.cond] in the C++ standard.

So, the representation with a VOID_TYPE is natural for C++.  That could
be lowered to some other representation at some point, but it should
not be changed in the C++ front end itself.  In general, there's no
good choice of expression to use for "y" in:

  b ? (throw 1, y) : x

I've missed some of this thread, but I'm not really sure why "void" is
not a reasonable choice for arms of conditionals that are guaranteed to
throw exceptions and/or not return.

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Nathan Sidwell]

Mark Mitchell wrote:

> Note that the C++ standard says that "throw 1" has type "void", and
> that it is OK to have a conditional expression like:
> 
>  b ? throw 1 : x

Yes, but note that
	b ? throw: x
has typeof (x), but
	b ? (void)0 : x
is invalid (unless x is also void).

Hmm, so all the reassociation machinery has to do is see if an arm has
void type, and then DTRT. That'd work for the no-return fn extension too.
But it does have the 'uncleanliness' of using void to say unreachable.

nathan

-- 
Dr Nathan Sidwell   ::   http://www.codesourcery.com   ::   CodeSourcery LLC
          'But that's a lie.' - 'Yes it is. What's your point?'
nathan@codesourcery.com : http://www.cs.bris.ac.uk/~nathan/ : nathan@acm.org

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Roger Sayle]

> >> Richard Henderson wrote:
> >> > Is this really necessary?  As a front-end thing, maybe, but it
> >> > would seem that the best way to handle this is to use
> >> >
> >> > 	(i >= 0 ? i : (abort(), 0))
> >> FE's must canonicalize 'cond ?: false' after all ...
> >
> > The patch that introduced these semantics (without documentation) was:
> > http://gcc.gnu.org/ml/gcc-patches/2001-02/msg01075.html
> >
> > 2001-02-17  Mark Mitchell  <mark@codesourcery.com>
> >
> >         * fold-const.c (fold_binary_op_with_conditional_arg): New
> >         function, split out from ...
> >         (fold): ... here.
> >         * tree.def (COND_EXPR): Document the use of VOID_TYPE for
> >         conditional arms that throw exceptions.
> >
> > which resolved PR c++/218.  In all fairness to Mark for adding this
> > functionality, has anyone actually tested whether Richards suggested
> > idiom "(i>=0 ? i : (abort(), 0))" currently works?
>
> I've missed some of this thread, but I'm not really sure why "void" is
> not a reasonable choice for arms of conditionals that are guaranteed to
> throw exceptions and/or not return.

Hi Mark,

Not surprising, this thread is actually the review of the following
patch:  http://gcc.gnu.org/ml/gcc-patches/2002-10/msg00873.html
"PR fortran/7388: VOID_TYPE_P in COND_EXPRs"

Shortly after this patch, which fixes constant folding of ?: in
the presence of children with void type and modifies g77 to use
this extension, I noticed the void type semantics of COND_EXPR
weren't documented hence the current thread.  The documentation
patch was approved, but whether these semantics are actually
needed is still in doubt.

I'd also appreciate it if someone could approve Nathan's patch in
this area, http://gcc.gnu.org/ml/gcc-patches/2002-10/msg01120.html,
which fixes PR c++/7209.  I suspect that there are more bugs here,
but I'm cautious to continue looking for an alternate solution to
PR fortran/7388, until the related fixes are in the tree.  We certainly
still need to fix "(abort(), 0)" irrespective of the void types in
COND_EXPR outcome.

Roger
--

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Mark Mitchell]

> I'd also appreciate it if someone could approve Nathan's patch in
> this area, http://gcc.gnu.org/ml/gcc-patches/2002-10/msg01120.html,

This patch is OK.  Thanks.

> We certainly still need to fix "(abort(), 0)" irrespective of the
> void types in COND_EXPR outcome.

Indeed.

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Toon Moene]

Roger Sayle wrote:

 >  I suspect that there are more bugs here,
> but I'm cautious to continue looking for an alternate solution to
> PR fortran/7388, until the related fixes are in the tree.  We certainly
> still need to fix "(abort(), 0)" irrespective of the void types in
> COND_EXPR outcome.

Roger, did you actually fix PR fortran/7388 ?  It doesn't fail on my 
powerpc-unknown-linux-gnu anymore and from the archive of 
gcc-testresults I get the impression it doesn't fail on a couple of 
other architectures anymore as well.

However, I can't find anything in the gcc and f ChangeLogs that 
indicates that you did apply a patch for 7388 ...

-- 
Toon Moene - mailto:toon@moene.indiv.nluug.nl - phoneto: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
Maintainer, GNU Fortran 77: http://gcc.gnu.org/onlinedocs/g77_news.html
Join GNU Fortran 95: http://g95.sourceforge.net/ (under construction)

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Roger Sayle]

On Sat, 26 Oct 2002, Toon Moene wrote:
> Roger, did you actually fix PR fortran/7388 ?  It doesn't fail on my
> powerpc-unknown-linux-gnu anymore and from the archive of
> gcc-testresults I get the impression it doesn't fail on a couple of
> other architectures anymore as well.

Hi Toon,

I'm sorry, I've effectively been off-line whilst mainline has been
unable to bootstrap on i686-pc-cygwin (PR bootstrap/8351), as thats
all I have at home.

However I wouldn't be surprised if PR fortran/7388 was fixed by
Nathan's patch for PR c++/7209.  This might have been all that
was needed to fix the remaining "(0, abort())" cases.

Roger
--

Re: [PATCH] Update COND_EXPR section of c-tree.texi

[Toon Moene]

Roger Sayle wrote:

> However I wouldn't be surprised if PR fortran/7388 was fixed by
> Nathan's patch for PR c++/7209.  This might have been all that
> was needed to fix the remaining "(0, abort())" cases.

Well, at least the "correct" test results start coming in from the 
moment Nathan's patch went in, so your guess is probably correct.

I've closed PR fortran/7388.

Thanks for your efforts in trying to solve it !

-- 
Toon Moene - mailto:toon@moene.indiv.nluug.nl - phoneto: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
Maintainer, GNU Fortran 77: http://gcc.gnu.org/onlinedocs/g77_news.html
Join GNU Fortran 95: http://g95.sourceforge.net/ (under construction)