[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-04 Thread aoliva at redhat dot com

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-04-04 
13:32 ---
Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold

On Apr  4, 2005, Roger Sayle [EMAIL PROTECTED] wrote:

   (fold_cond_expr_with_comparison): Preserve lvalue-ness for the
   C++ front-end prior to lowering into gimple form.

   * expr2.C: Fixed.

Err...  Why did you choose to drop the portion of the patch below,
that would have avoided the ugliness of comparing langhooks.name, but
still retained it in the ChangeLog entry?

Index: fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.554
diff -u -p -d -u -p -d -u -p -r1.554 fold-const.c
--- fold-const.c4 Apr 2005 08:50:25 -   1.554
+++ fold-const.c4 Apr 2005 13:25:47 -
@@ -4173,7 +4173,15 @@ fold_cond_expr_with_comparison (tree typ
   tree arg00 = TREE_OPERAND (arg0, 0);
   tree arg01 = TREE_OPERAND (arg0, 1);
   tree arg1_type = TREE_TYPE (arg1);
-  tree tem;
+  tree tem = NULL;
+  /* If the COND_EXPR can possibly be an lvalue, we don't want to
+ perform transformations that return a simplified result that will
+ be recognized as lvalue, but that will not match the expected
+ result.  We may still return other expressions that would be
+ incorrect, but those are going to be rvalues, and the caller is
+ supposed to discard them.  */
+  bool lvalue = !pedantic_lvalues
+ maybe_lvalue_p (arg1)  maybe_lvalue_p (arg2);
 
   STRIP_NOPS (arg1);
   STRIP_NOPS (arg2);
@@ -4215,10 +4223,12 @@ fold_cond_expr_with_comparison (tree typ
   case EQ_EXPR:
   case UNEQ_EXPR:
tem = fold_convert (arg1_type, arg1);
-   return pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+   tem = pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+   break;
   case NE_EXPR:
   case LTGT_EXPR:
-   return pedantic_non_lvalue (fold_convert (type, arg1));
+   tem = pedantic_non_lvalue (fold_convert (type, arg1));
+   break;
   case UNGE_EXPR:
   case UNGT_EXPR:
if (flag_trapping_math)
@@ -4230,7 +4240,8 @@ fold_cond_expr_with_comparison (tree typ
  arg1 = fold_convert (lang_hooks.types.signed_type
   (TREE_TYPE (arg1)), arg1);
tem = fold_build1 (ABS_EXPR, TREE_TYPE (arg1), arg1);
-   return pedantic_non_lvalue (fold_convert (type, tem));
+   tem = pedantic_non_lvalue (fold_convert (type, tem));
+   break;
   case UNLE_EXPR:
   case UNLT_EXPR:
if (flag_trapping_math)
@@ -4241,12 +4252,18 @@ fold_cond_expr_with_comparison (tree typ
  arg1 = fold_convert (lang_hooks.types.signed_type
   (TREE_TYPE (arg1)), arg1);
tem = fold_build1 (ABS_EXPR, TREE_TYPE (arg1), arg1);
-   return negate_expr (fold_convert (type, tem));
+   tem = negate_expr (fold_convert (type, tem));
+   break;
   default:
gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison);
break;
   }
 
+  if (tem  (! lvalue || maybe_lvalue_p (tem)))
+return tem;
+  else
+tem = NULL;
+
   /* A != 0 ? A : 0 is simply A, unless A is -0.  Likewise
  A == 0 ? A : 0 is always 0 unless A is -0.  Note that
  both transformations are correct when A is NaN: A != 0
@@ -4255,11 +4272,16 @@ fold_cond_expr_with_comparison (tree typ
   if (integer_zerop (arg01)  integer_zerop (arg2))
 {
   if (comp_code == NE_EXPR)
-   return pedantic_non_lvalue (fold_convert (type, arg1));
+   tem = pedantic_non_lvalue (fold_convert (type, arg1));
   else if (comp_code == EQ_EXPR)
-   return fold_convert (type, integer_zero_node);
+   tem = fold_convert (type, integer_zero_node);
 }
 
+  if (tem  (! lvalue || maybe_lvalue_p (tem)))
+return tem;
+  else
+tem = NULL;
+
   /* Try some transformations of A op B ? A : B.
 
  A == B? A : Bsame as B
@@ -4309,9 +4331,15 @@ fold_cond_expr_with_comparison (tree typ
   switch (comp_code)
{
case EQ_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg2));
+ if (lvalue)
+   break;
+ tem = pedantic_non_lvalue (fold_convert (type, arg2));
+ break;
case NE_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ if (lvalue)
+   break;
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
+ break;
case LE_EXPR:
case LT_EXPR:
case UNLE_EXPR:
@@ -4327,7 +4355,7 @@ fold_cond_expr_with_comparison (tree typ
  tem = (comp_code == LE_EXPR || comp_code == UNLE_EXPR)
? fold_build2 (MIN_EXPR, comp_type, comp_op0, comp_op1)
: fold_build2 (MIN_EXPR, comp_type, comp_op1, comp_op0);
- return pedantic_non_lvalue (fold_convert (type, tem));
+ tem = 

[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-04 Thread joseph at codesourcery dot com

--- Additional Comments From joseph at codesourcery dot com  2005-04-04 
13:41 ---
Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold

On Mon, 4 Apr 2005, Alexandre Oliva wrote:

 Err...  Why did you choose to drop the portion of the patch below,
 that would have avoided the ugliness of comparing langhooks.name, but
 still retained it in the ChangeLog entry?

pedantic_lvalues means this is C, until I manage to disentangle enough 
C/C++/ObjC interactions to keep lvalueness internal to the C front end so 
pedantic_lvalues can go away.  Checking langhooks.name tests for C++, 
i.e. they differ in their effects for all languages other than C/ObjC/C++: 
only C/ObjC require that conditional expressions not be turned into 
lvalues if they weren't such to start with, while only C++ requires that 
conditional expressions which were lvalues be preserved in their original 
form.  The proper hook to check is one meaning this is C++ rather than 
this is not C, though perhaps adding a new hook 
fold_preserve_cond_expr_p would be cleaner than checking langhooks.name.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-04 Thread aoliva at redhat dot com

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-04-04 
13:50 ---
Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold

On Apr  4, 2005, Alexandre Oliva [EMAIL PROTECTED] wrote:

 On Apr  4, 2005, Roger Sayle [EMAIL PROTECTED] wrote:

 long-term solutions have been proposed for g++ 4.1, the patch below is
 the best 4.0 timeframe work-around that doesn't adversely affect
 performance for GCC's non C++ front-ends,

 I don't understand your claim.  My patch used pedantic_lvalues, which
 is set in such a way by the C and C++ front-ends that makes it even
 faster than the string comparison you're using.

 and even retains the ability to synthesize MIN_EXPR and MAX_EXPR for
 C++ during the tree-ssa passes.

 This was never removed in my latest patch.

BTW, here's a patch for mainline that removes the bogus comment the
patch you checked in introduced, and adjusts the lvalueness tests such
that they're performed early, and used at any point we need to make a
decision on whether the transformation is valid.  At the point you
perform the test, we may have already stripped some nop_exprs, which
might make rvalues look like lvalues, removing some optimization
possibilities.

Would you like me to add the langhook to this, or do you oppose the
entire approach for some reason you still haven't explained?

Index: gcc/ChangeLog
from  Alexandre Oliva  [EMAIL PROTECTED]

PR c++/19199
* fold-const.c (maybe_lvalue_p): Remove bogus comment introduced
in previous patch.
(fold_cond_expr_with_comparison): Determine whether lvalueness is
needed upfront, and proceed accordingly in each case.

Index: gcc/fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.554
diff -u -p -r1.554 fold-const.c
--- gcc/fold-const.c 4 Apr 2005 08:50:25 - 1.554
+++ gcc/fold-const.c 4 Apr 2005 13:42:43 -
@@ -2005,7 +2005,6 @@ fold_convert (tree type, tree arg)
 
 /* Return false if expr can be assumed not to be an value, true
otherwise.  */
-/* Return an expr equal to X but certainly not valid as an lvalue.  */
 
 static bool
 maybe_lvalue_p (tree x)
@@ -4173,7 +4172,15 @@ fold_cond_expr_with_comparison (tree typ
   tree arg00 = TREE_OPERAND (arg0, 0);
   tree arg01 = TREE_OPERAND (arg0, 1);
   tree arg1_type = TREE_TYPE (arg1);
-  tree tem;
+  tree tem = NULL;
+  /* If the COND_EXPR can possibly be an lvalue, we don't want to
+ perform transformations that return a simplified result that will
+ be recognized as lvalue, but that will not match the expected
+ result.  We may still return other expressions that would be
+ incorrect, but those are going to be rvalues, and the caller is
+ supposed to discard them.  */
+  bool lvalue = !pedantic_lvalues
+ maybe_lvalue_p (arg1)  maybe_lvalue_p (arg2);
 
   STRIP_NOPS (arg1);
   STRIP_NOPS (arg2);
@@ -4215,10 +4222,12 @@ fold_cond_expr_with_comparison (tree typ
   case EQ_EXPR:
   case UNEQ_EXPR:
tem = fold_convert (arg1_type, arg1);
-   return pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+   tem = pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+   break;
   case NE_EXPR:
   case LTGT_EXPR:
-   return pedantic_non_lvalue (fold_convert (type, arg1));
+   tem = pedantic_non_lvalue (fold_convert (type, arg1));
+   break;
   case UNGE_EXPR:
   case UNGT_EXPR:
if (flag_trapping_math)
@@ -4230,7 +4239,8 @@ fold_cond_expr_with_comparison (tree typ
  arg1 = fold_convert (lang_hooks.types.signed_type
   (TREE_TYPE (arg1)), arg1);
tem = fold_build1 (ABS_EXPR, TREE_TYPE (arg1), arg1);
-   return pedantic_non_lvalue (fold_convert (type, tem));
+   tem = pedantic_non_lvalue (fold_convert (type, tem));
+   break;
   case UNLE_EXPR:
   case UNLT_EXPR:
if (flag_trapping_math)
@@ -4241,12 +4251,18 @@ fold_cond_expr_with_comparison (tree typ
  arg1 = fold_convert (lang_hooks.types.signed_type
   (TREE_TYPE (arg1)), arg1);
tem = fold_build1 (ABS_EXPR, TREE_TYPE (arg1), arg1);
-   return negate_expr (fold_convert (type, tem));
+   tem = negate_expr (fold_convert (type, tem));
+   break;
   default:
gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison);
break;
   }
 
+  if (tem  (! lvalue || maybe_lvalue_p (tem)))
+return tem;
+  else
+tem = NULL;
+
   /* A != 0 ? A : 0 is simply A, unless A is -0.  Likewise
  A == 0 ? A : 0 is always 0 unless A is -0.  Note that
  both transformations are correct when A is NaN: A != 0
@@ -4255,11 +4271,16 @@ fold_cond_expr_with_comparison (tree typ
   if (integer_zerop (arg01)  integer_zerop (arg2))
 {
   if (comp_code == NE_EXPR)
-   return pedantic_non_lvalue (fold_convert (type, arg1));
+   tem = 

[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-04 Thread aoliva at redhat dot com

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-04-04 
15:02 ---
Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold

On Apr  4, 2005, Alexandre Oliva [EMAIL PROTECTED] wrote:

 + result.  We may still return other expressions that would be
 + incorrect, but those are going to be rvalues, and the caller is
 + supposed to discard them.  */

Uhh...  This sentence is no longer true.  I'm removing it from my
local copy of the patch.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-04 Thread roger at eyesopen dot com

--- Additional Comments From roger at eyesopen dot com  2005-04-04 16:02 
---
Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold


Hi Alex,

My apologies yet again for not being more explicit about all of the
things that were wrong (and or I was unhappy with) with your proposed
solution.  I'd hoped that I was clear in the comments in the bugzilla
thread, and that you'd appreciate the issues it addressed.

Problems with your approach:

[1] The use of pedantic_lvalues to identify the non-C front-ends,
adversely affects code generation in the Java, Fortran and Ada
front-ends.  The use of COND_EXPRs as lvalues is unique to the
C++ front-end, so ideally a fix shouldn't regress code quality on
innocent front-ends.  Certainly, not without benchmarking to
indicate how significant a performance hit, these other languages
are taking prior to a release.

[2] The pedantic_lvalues flag is itself a hack used by the C
front-end, that is currently being removed by Joseph in his clean-up
of the C parser.  Adding this use would block his efforts, until an
alternate solution is found.  Admittedly, this isn't an issue for
the 4.0 release, but creates more work or a regression once this is
removed from mainline.

[3] Your patch is too invasive.  Compared to the four-line counter
proposal that disables just the problematic class of transformations,
your much larger patch inherently contains a much larger risk.  For
example, there is absolutely no need to modify the source code on the
A = 0 ? A : -A  -   abs (A) paths as these transformations could
never interfere with the lvalueness of an expression.

Additionally, once one of the longer term solutions proposed by Mark
or me is implemented, all of these workarounds will have to be undone/
reverted.  By only affecting a single clause, we avoid the potential
for leaving historic code bit-rotting in the tree.

[4] Despite your counter claims you approach *does* inhibit the ability
of the the tree-ssa optimizers to synthesize MIN_EXPR and MAX_EXPR
nodes.  Once the in_gimple_form flag is set, fold-const.c is able to
optimize A == B ? A : B  -  B even when compiling C++, as it knows
that a COND_EXPR can't be used as an lvalue in the late middle-end.

[5] For the immediate term, I don't think its worth worrying about
converting non-lvalues into lvalues, the wrong-code bugs and diagnostic
issues are related solely to the lvalue - non-lvalue transition.
At this stage of pre-release, lower risk changes are cleary preferrable,
and making this change will break code that may have erroneously compiled
in the past.  Probably, OK for 4.0, but not for 3.4 (which also exhibits
this problem).


And although, not serious enough to warrant a [6], it should be pointed
out that several of your recent patches have introduced regressions.
Indeed, you've not yet reported that your patch has been sucessfully
bootstraped or regression tested on any target triple.  Indeed, your
approach of posting a patch before completing the prerequisite testing
sprecified/stressed in contribute.html, on more than one occassion
recently resulted in the patch having to be rewritten/tweaked.  Indeed,
as witnessed in comment #17, I've already approved an earlier version
your patch once, only to later discover you were wasting my time.


As a middle-end maintainer, having been pinged by the release manager
that we/you weren't making sufficient progress towards addressing the
issues with your patch, I took the opportunity to apply a fix that
was within my authority to commit.  If you'd had made and tested the
changes that I requested in a timely manner, I'm sure I'd have approved
those efforts by now.

My apologies again for not being blunt earlier.  My intention was
that by listing all of the benefits of an alternate approach in
comment #19 of the bugzilla PR, I wouldn't have to explicitly list
them as the defficiencies of your approach.  Some people prefer the
carrot to the stick with patch reviews [others like RTH's No].


Perhaps, I should ask the counter question to your comment #21?
In what way do you feel that the committed patch isn't clearly
superior to your proposed solution?  p.s. Thanks for spotting my
mistake of leaving a bogus comment above maybe_lvalue_p.

Roger
--



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-04 Thread mark at codesourcery dot com

--- Additional Comments From mark at codesourcery dot com  2005-04-04 16:39 
---
Subject: Re:  [3.3/3.4/4.0/4.1 Regression] Wrong warning about
 returning a reference to a temporary

Roger --

Thank you for fixing this PR!  Very much appreciated.

If I'm reading correctly, the patch is now on the mainline, so the 4.1 
regression listing in the subject line is now incorrect?  Your patch is 
definitely suitable for 4.0 as well; I shall look forward to seeing it 
there.

Thanks,



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-03 Thread mark at codesourcery dot com

--- Additional Comments From mark at codesourcery dot com  2005-04-04 00:37 
---
Subject: Re:  [3.3/3.4/4.0/4.1 Regression] Wrong warning about
 returning a reference to a temporary

roger at eyesopen dot com wrote:

 I'd hoped I'd made this clear when I proposed the alternate strategy of only
 disabling this optimization *only* in the C++ front-end, and *only* prior to
 tree-ssa.

Roger, Alexandre, will you to please coordinate to determine who will 
take responsibility for preparing a patch?

What's difficult for me is that I can't quite figure out who's going to 
actually take this bug and run with it.  I think Alexandre is trying 
hard to fix it, but I'm not confident that he understands what Roger 
wants.  If you two aren't able to work through the issues, or don't have 
time, please let me know; I want to find some way to get this problem 
dealt with before 4.0.

Thanks,



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-03 Thread cvs-commit at gcc dot gnu dot org

--- Additional Comments From cvs-commit at gcc dot gnu dot org  2005-04-04 
05:02 ---
Subject: Bug 19199

CVSROOT:/cvs/gcc
Module name:gcc
Changes by: [EMAIL PROTECTED]   2005-04-04 05:02:10

Modified files:
gcc: ChangeLog fold-const.c 
gcc/testsuite  : ChangeLog 
gcc/testsuite/g++.old-deja/g++.oliva: ChangeLog expr2.C 
Added files:
gcc/testsuite/g++.dg/expr: lval2.C 

Log message:
2005-04-03  Roger Sayle  [EMAIL PROTECTED]
Alexandre Oliva  [EMAIL PROTECTED]

PR c++/19199
* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_cond_expr_with_comparison): Preserve lvalue-ness for the
C++ front-end prior to lowering into gimple form.

* g++.dg/expr/lval2.C: New.

* expr2.C: Fixed.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gccr1=2.8104r2=2.8105
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fold-const.c.diff?cvsroot=gccr1=1.552r2=1.553
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gccr1=1.5273r2=1.5274
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/expr/lval2.C.diff?cvsroot=gccr1=NONEr2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.old-deja/g++.oliva/ChangeLog.diff?cvsroot=gccr1=1.35r2=1.36
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.old-deja/g++.oliva/expr2.C.diff?cvsroot=gccr1=1.5r2=1.6



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-02 Thread aoliva at redhat dot com

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-04-02 
17:29 ---
Subject: Re:  [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a 
reference to a temporary

On Mar  9, 2005, Alexandre Oliva [EMAIL PROTECTED] wrote:

   PR c++/19199
   * fold-const.c (non_lvalue): Split tests into...
   (maybe_lvalue_p): New function.
   (fold_cond_expr_with_comparison): Preserve lvalue-ness.

Ping?

http://gcc.gnu.org/PR19199

I have further local changes to adjust for other changes that went in,
that I'll post upon request, or when the patch goes in.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-04-02 Thread roger at eyesopen dot com

--- Additional Comments From roger at eyesopen dot com  2005-04-03 03:20 
---
 Excuse me for asking, but what is it that makes the latest patch I posted not
 reasonable for the 4.0 timeframe?

The performance regression on C, Java, Ada and fortran code, that isn't affected
by this bug.  The bug is marked has the c++ component because it only affects
the C++ front-end.  A fix that disables MIN_EXPR/MAX_EXPR optimizations in all
front-ends is not suitable for a release branch without SPEC testing to show how
badly innocent targets will get burnt!  There may be lots of places in the Linux
kernel that depend upon generating min/max insns for performance reasons...

I'd hoped I'd made this clear when I proposed the alternate strategy of only
disabling this optimization *only* in the C++ front-end, and *only* prior to
tree-ssa.  Whilst I agree this is a serious bug, it currently affects all
release branches, so taking a universal performance hit to resolve it without
considering the consequences seems a bad decision.  Just grep for MIN ( and
MAX ( in GCC's own source code to see how badly this could impact the
compiler's own code/performance/bootstrap times.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-29 Thread aoliva at gcc dot gnu dot org

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-30 
02:55 ---
Excuse me for asking, but what is it that makes the latest patch I posted not
reasonable for the 4.0 timeframe?

-- 
   What|Removed |Added

 AssignedTo|unassigned at gcc dot gnu   |aoliva at gcc dot gnu dot
   |dot org |org
 Status|NEW |ASSIGNED
   Last reconfirmed|2004-12-30 14:48:58 |2005-03-30 02:55:13
   date||


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-29 Thread mark at codesourcery dot com

--- Additional Comments From mark at codesourcery dot com  2005-03-30 07:20 
---
Subject: Re:  [3.3/3.4/4.0/4.1 Regression] Wrong warning about
 returning a reference to a temporary

aoliva at gcc dot gnu dot org wrote:
 --- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-30 
 02:55 ---
 Excuse me for asking, but what is it that makes the latest patch I posted not
 reasonable for the 4.0 timeframe?

I'm not sure if there is anything that makes it unreasonable; I don't 
know either way.  I hope Roger will comment.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-24 Thread roger at eyesopen dot com

--- Additional Comments From roger at eyesopen dot com  2005-03-25 06:03 
---
Splitting non_value into maybe_lvalue_p is a good thing, is totally safe and is
preapproved for both mainline and the 4.0 branch.  The remaining change to
fold_ternary/fold_cond_expr_with_comparison are more controversial, and can
theoretically be discussed independently.

Reading through each of the transformations of COND_EXPR in fold, all of the
problematic transformations are guarded in the block beginning at line 4268
of fold-const.c.  This is the set of A op B ? A : B transformations.  All
other transformations are either lvalue-safe, or require either operand 2 or
operand 3 to be a non-lvalue (typically operand 3 must be a constant).

I believe a suitable 4.0-timescale (grotesque hack workaround) is (untested):

--- 4265,4275 
   a number and A is not.  The conditions in the original
   expressions will be false, so all four give B.  The min()
   and max() versions would give a NaN instead.  */
!   if (operand_equal_for_comparison_p (arg01, arg2, arg00)
!(in_gimple_form
!   || strcmp (lang_hooks.name, GNU C++) != 0
!   || ! maybe_lvalue_p (arg1)
!   || ! maybe_lvalue_p (arg2)))
  {
tree comp_op0 = arg00;
tree comp_op1 = arg01;

The maybe_lvalue_p tests should be obvious from the previous versions of
Alexandre's patch.  The remaining two lines are both hideous hacks.  The
first is that these transformations only need to be disabled for the C++
front-end.  This is (AFAIK) the only language front-end that uses COND_EXPRs
as lvalues, and disabling x  y ? x : y into MAX_EXPR (where x and y are
VAR_DECLs) is less than ideal for the remaining front-ends.  The second equally
bad hack is to re-allow this transformation even for C++ once we're in the
tree-ssa optimizers.  I believe once we're in gimple, COND_EXPR is no longer
allowed as the lhs of an assignment, hence the MAX_EXPR/MIN_EXPR recognition
transformations are the gimple-level should be able to clean-up any rvalue
COND_EXPRs they're presented with.

Clearly, testing lang_hooks.name is an option of last resort.  I'm increasingly
convinced that the correct long term solution is to introduce a new LCOND_EXPR
tree node for use by the C++ end.  Either as a C++-only tree code, or a generic
tree code.  Additionally, depending upon whether we go ahead and deprecate ?=
and ?= operators, we may or may not also require LMIN_EXPR and LMAX_EXPR. This
allows us to correctly separate the semantics: MIN_EXPR is commutative,
LMIN_EXPR isn't, MIN_EXPR is not an lvalue, LMIN_EXPR is.  If either operand
of a L*_EXPR is not an lvalue, it can be folded to the rvalue form.  The
problematic transformations above can then be controlled via COND_EXPR vs.
LCOND_EXPR rather than checking the front-end.  cp/call.c's
build_conditional_expr is then  tweaked to use LCOND_EXPR instead of COND_EXPR.
Finally either with tweaks to the grammar, gimplification or g++'s other
tree manipulation machinery we can introduce a lower_to_rvalue call to
recursively convert LCOND_EXPR into COND_EXPR, as soon as we know the
expression can't be used as an lvalue.  [Aside: In fact, a fold_rvalue
could be used by the middle-end, to additionally handle cases such as
const_array[0] which can't currently be handled due to the fear of being
used in an lvalue-context, such as (const_array[0]).

Whilst I see LCOND_EXPR (and maybe LMIN_EXPR, LMAX_EXPR) as the long-term
way to go, this is unquestionably too disruptive for the gcc-4_0-branch.
This leaves us with two options, either suffer an rare miscompilation  or
take a performance hit in the sake of correctness.  I'm in favour of the
latter provided we don't unfairly impact the C/fortran/ada and Java front-ends
that aren't otherwise affected.  The above changes even attempt to allow the
std::min and std::max operators to continue to be optimized, in an attempt to
limit the performance impact to pre-gimple tree-folding during C++ parsing.

 
Alex, could you confirm that the above suggestion resolves the PR when used
in combination with your maybe_lvalue_p split?  Mark, do you agree that this
is a reasonable (the most reasonable?) compromise in the 4.0 timeframe?  I'm
not proud of querying the front-end in fold-const.c, but the semantics
required of COND_EXPR by the C++ front-end conflict with those of the other
front-ends.  Normally this means different tree codes, but that's a luxury
I don't think we can currently afford.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-24 Thread mark at codesourcery dot com

--- Additional Comments From mark at codesourcery dot com  2005-03-25 06:29 
---
Subject: Re:  [3.3/3.4/4.0/4.1 Regression] Wrong warning about
 returning a reference to a temporary


 Alex, could you confirm that the above suggestion resolves the PR when used
 in combination with your maybe_lvalue_p split?  Mark, do you agree that this
 is a reasonable (the most reasonable?) compromise in the 4.0 timeframe? 

Thank you for looking at this again.

Yes, I think your patch is a reasonable solution in the 4.0 timeframe. 
It does, of course, mean that there will be programs that are less 
well-optimized in C++ than in C, but I agree with you that there's no 
good way around that in the short term.  If you're worried about the 
cost of the strcmp, you could cache the result, but I have no reason to 
think that's a real problem.  Also, I think this patch should go on the 
mainline too, until we get a better fix -- but I think we should work on 
that, so that this doesn't actually make it into 4.1.

(In 4.1, I disagree that we need new tree codes.  I think all we need to 
do is wait to call fold until gimplification time in C++; then, the 
middle end will never see COND_EXPRs used as lvalues.  In C++, 
conditionals can be lvalues, but we actually fix that up before passing 
them to the middle end.  The middle end sees COND_EXPRs that would be 
lvalues in C++, but they are only used as rvalues, so the transformation 
in fold would be fine.

But, we can have that debate later!)

Thanks,



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-08 Thread aoliva at redhat dot com

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-08 
23:23 ---
Subject: Re:  [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a 
reference to a temporary

On Mar  7, 2005, Roger Sayle [EMAIL PROTECTED] wrote:

 For example, I believe that Alex's proposed solution to PR c++/19199
 isn't an appropriate fix.  It's perfectly reasonable for fold to convert
 a C++ COND_EXPR into a MIN_EXPR or MAX_EXPR, as according to the C++
 front-end all three of these tree nodes are valid lvalues.  Hence it's
 not this transformation in fold that's in error.

Bugzilla was kept out of the long discussion that ensued, so I'll try
to summarize.  The problem is that the conversion is turning a
COND_EXPR such as:

  ((int)a  (int)b ? a : b)

into

  (__typeof(a)) ((int)a ? (int)b)

which is not an lvalue.  This is the transformation we have to avoid.


 Simply disabling the COND_EXPR - MIN_EXPR/MAX_EXPR transformation is
 also likely to be a serious performance penalty, especially on targets
 that support efficient sequences for min and max.

This was not what I intended to do with my patch, FWIW.
Unfortunately, I goofed in the added call to operand_equal_p, limiting
too much the situations in which the optimization could be applied.
The patch fixes this problem, and updates the patch such that it
applies cleanly again.

As for other languages whose COND_EXPRs can't be lvalues, we can
probably arrange quite easily for them to ensure at least one of their
result operands is not an lvalue, so as to enable the transformation
again.

Comments?  Ok to install?

Index: gcc/ChangeLog
from  Alexandre Oliva  [EMAIL PROTECTED]

* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_ternary): Use it to avoid turning a COND_EXPR lvalue into
a MIN_EXPR rvalue.

Index: gcc/fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.535
diff -u -p -r1.535 fold-const.c
--- gcc/fold-const.c 7 Mar 2005 21:24:21 - 1.535
+++ gcc/fold-const.c 8 Mar 2005 22:07:52 -
@@ -2005,16 +2005,12 @@ fold_convert (tree type, tree arg)
 }
 }
 
-/* Return an expr equal to X but certainly not valid as an lvalue.  */
+/* Return false if expr can be assumed to not be an lvalue, true
+   otherwise.  */
 
-tree
-non_lvalue (tree x)
+static bool
+maybe_lvalue_p (tree x)
 {
-  /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
- us.  */
-  if (in_gimple_form)
-return x;
-
   /* We only need to wrap lvalue tree codes.  */
   switch (TREE_CODE (x))
   {
@@ -2054,8 +2050,24 @@ non_lvalue (tree x)
 /* Assume the worst for front-end tree codes.  */
 if ((int)TREE_CODE (x) = NUM_TREE_CODES)
   break;
-return x;
+return false;
   }
+
+  return true;
+}
+
+/* Return an expr equal to X but certainly not valid as an lvalue.  */
+
+tree
+non_lvalue (tree x)
+{
+  /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
+ us.  */
+  if (in_gimple_form)
+return x;
+
+  if (! maybe_lvalue_p (x))
+return x;
   return build1 (NON_LVALUE_EXPR, TREE_TYPE (x), x);
 }
 
@@ -9734,10 +9746,16 @@ fold_ternary (tree expr)
 of B and C.  Signed zeros prevent all of these transformations,
 for reasons given above each one.
 
+We don't want to use operand_equal_for_comparison_p here,
+because it might turn an lvalue COND_EXPR into an rvalue one,
+see PR c++/19199.
+
  Also try swapping the arguments and inverting the conditional.  */
   if (COMPARISON_CLASS_P (arg0)
-  operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
-arg1, TREE_OPERAND (arg0, 1))
+  ((maybe_lvalue_p (op1)  maybe_lvalue_p (op2))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0), op1, 0)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+   arg1, TREE_OPERAND (arg0, 1)))
   !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg1
{
  tem = fold_cond_expr_with_comparison (type, arg0, op1, op2);
@@ -9746,9 +9764,10 @@ fold_ternary (tree expr)
}
 
   if (COMPARISON_CLASS_P (arg0)
-  operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
-op2,
-TREE_OPERAND (arg0, 1))
+  ((maybe_lvalue_p (op1)  maybe_lvalue_p (op2))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0), op2, 0)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+   op2, TREE_OPERAND (arg0, 1)))
   !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (op2
{
  tem = invert_truthvalue (arg0);
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  [EMAIL PROTECTED]

PR c++/19199
* 

[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-08 Thread roger at eyesopen dot com

--- Additional Comments From roger at eyesopen dot com  2005-03-09 01:28 
---
Subject: Re:  [3.3/3.4/4.0/4.1 Regression] Wrong warning about
 returning a reference to a temporary


On 8 Mar 2005, Alexandre Oliva wrote:

   * fold-const.c (non_lvalue): Split tests into...
   (maybe_lvalue_p): New function.
   (fold_ternary): Use it to avoid turning a COND_EXPR lvalue into
   a MIN_EXPR rvalue.

This version is Ok for mainline, and currently open release branches.

Thanks,

Roger
--



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-08 Thread aoliva at redhat dot com

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-09 
04:11 ---
Subject: Re:  [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a 
reference to a temporary

On Mar  8, 2005, Roger Sayle [EMAIL PROTECTED] wrote:

 On 8 Mar 2005, Alexandre Oliva wrote:
 
 * fold-const.c (non_lvalue): Split tests into...
 (maybe_lvalue_p): New function.
 (fold_ternary): Use it to avoid turning a COND_EXPR lvalue into
 a MIN_EXPR rvalue.

 This version is Ok for mainline, and currently open release branches.

Unfortunately, the problem in g++.oliva/expr2.C resurfaced, because,
as it turns out, the transformation (I != J ? I : J) = I yields an
lvalue as required, but not the *correct* lvalue in all cases.  I
tried what you'd suggested on IRC (testing whether the thing is an
lvalue after the fact), but that obviously failed in this case as
well.

So I figured a better approach would be to perform lvalue tests to
fold_cond_expr_with_comparison(), where we can avoid specific
inappropriate transformations while still trying other alternatives.

While at that, I figured we could use pedantic_lvalues to avoid
refraining from applying optimizations just because it looks like the
cond-expr might be an lvalue, even though the language requires it not
to be.

This is currently bootstrapping on x86_64-linux-gnu.  Ok to install if
bootstrap completes and regtesting passes?

Index: gcc/ChangeLog
from  Alexandre Oliva  [EMAIL PROTECTED]

PR c++/19199
* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_cond_expr_with_comparison): Preserve lvalue-ness.

Index: gcc/fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.535
diff -u -p -r1.535 fold-const.c
--- gcc/fold-const.c 7 Mar 2005 21:24:21 - 1.535
+++ gcc/fold-const.c 9 Mar 2005 03:59:18 -
@@ -2005,16 +2005,12 @@ fold_convert (tree type, tree arg)
 }
 }
 
-/* Return an expr equal to X but certainly not valid as an lvalue.  */
+/* Return false if expr can be assumed to not be an lvalue, true
+   otherwise.  */
 
-tree
-non_lvalue (tree x)
+static bool
+maybe_lvalue_p (tree x)
 {
-  /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
- us.  */
-  if (in_gimple_form)
-return x;
-
   /* We only need to wrap lvalue tree codes.  */
   switch (TREE_CODE (x))
   {
@@ -2054,8 +2050,24 @@ non_lvalue (tree x)
 /* Assume the worst for front-end tree codes.  */
 if ((int)TREE_CODE (x) = NUM_TREE_CODES)
   break;
-return x;
+return false;
   }
+
+  return true;
+}
+
+/* Return an expr equal to X but certainly not valid as an lvalue.  */
+
+tree
+non_lvalue (tree x)
+{
+  /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
+ us.  */
+  if (in_gimple_form)
+return x;
+
+  if (! maybe_lvalue_p (x))
+return x;
   return build1 (NON_LVALUE_EXPR, TREE_TYPE (x), x);
 }
 
@@ -4162,7 +4174,15 @@ fold_cond_expr_with_comparison (tree typ
   tree arg00 = TREE_OPERAND (arg0, 0);
   tree arg01 = TREE_OPERAND (arg0, 1);
   tree arg1_type = TREE_TYPE (arg1);
-  tree tem;
+  tree tem = NULL;
+  /* If the COND_EXPR can possibly be an lvalue, we don't want to
+ perform transformations that return a simplified result that will
+ be recognized as lvalue, but that will not match the expected
+ result.  We may still return other expressions that would be
+ incorrect, but those are going to be rvalues, and the caller is
+ supposed to discard them.  */
+  bool lvalue = !pedantic_lvalues
+ maybe_lvalue_p (arg1)  maybe_lvalue_p (arg2);
 
   STRIP_NOPS (arg1);
   STRIP_NOPS (arg2);
@@ -4196,10 +4216,12 @@ fold_cond_expr_with_comparison (tree typ
   case EQ_EXPR:
   case UNEQ_EXPR:
tem = fold_convert (arg1_type, arg1);
-   return pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+   tem = pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+   break;
   case NE_EXPR:
   case LTGT_EXPR:
-   return pedantic_non_lvalue (fold_convert (type, arg1));
+   tem = pedantic_non_lvalue (fold_convert (type, arg1));
+   break;
   case UNGE_EXPR:
   case UNGT_EXPR:
if (flag_trapping_math)
@@ -4211,7 +4233,8 @@ fold_cond_expr_with_comparison (tree typ
  arg1 = fold_convert (lang_hooks.types.signed_type
   (TREE_TYPE (arg1)), arg1);
tem = fold (build1 (ABS_EXPR, TREE_TYPE (arg1), arg1));
-   return pedantic_non_lvalue (fold_convert (type, tem));
+   tem = pedantic_non_lvalue (fold_convert (type, tem));
+   break;
   case UNLE_EXPR:
   case UNLT_EXPR:
if (flag_trapping_math)
@@ -4222,12 +4245,18 @@ fold_cond_expr_with_comparison (tree typ
  arg1 = fold_convert (lang_hooks.types.signed_type
   (TREE_TYPE (arg1)), arg1);
tem = fold (build1 (ABS_EXPR, 

[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-06 Thread aoliva at redhat dot com

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-07 
03:28 ---
Subject: Re: [PR c++/19199] don't turn cond_expr lvalue into min_expr rvalue 
(continued from PR c++/20280)

On Mar  5, 2005, Mark Mitchell [EMAIL PROTECTED] wrote:

 Roger has objected to this change in the past.  As I noted in the
 audit trail for 19199, there's stuff in build_modify_expr to handle
 MIN_EXPR/MAX_EXPR as lvalues

Problem is, with the transformation performed by fold, it's no longer
an lvalue, and forcing it back into an lvalue just because it looks
like it should be one could break other programs.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-06 Thread mark at codesourcery dot com

--- Additional Comments From mark at codesourcery dot com  2005-03-07 04:19 
---
Subject: Re: [PR c++/19199] don't turn cond_expr lvalue into min_expr rvalue
 (continued from PR c++/20280)

Alexandre Oliva wrote:
 On Mar  5, 2005, Mark Mitchell [EMAIL PROTECTED] wrote:
 
 
Roger has objected to this change in the past.  As I noted in the
audit trail for 19199, there's stuff in build_modify_expr to handle
MIN_EXPR/MAX_EXPR as lvalues
 
 
 Problem is, with the transformation performed by fold, it's no longer
 an lvalue, and forcing it back into an lvalue just because it looks
 like it should be one could break other programs.

Yes, I understand.  You still need to take it up with Roger, though.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-05 Thread mark at codesourcery dot com

--- Additional Comments From mark at codesourcery dot com  2005-03-06 00:14 
---
Subject: Re: [PR c++/19199] don't turn cond_expr lvalue into min_expr rvalue
(continued from PR c++/20280)

Alexandre Oliva wrote:

 Here's a patch that fixes PR c++/19199, by avoiding the transformation
 from:
 
 cond lt nop(int) parm a  nop(int) parm b  
   parm a parm b 
 
 into:
 
 nop(enum) min nop(int) parm a  nop(int) parm b  
 
 since the latter is clearly not an lvalue, and
 rationalize_conditional_expr doesn't manage to turn it back into one
 (I don't think it should).  I realize we might miss some optimization
 opportunities because of this change in C, because the optimization
 would be valid there, but I suppose we could arrange for cond_expr
 operands in C to be forced into rvalues.

Roger has objected to this change in the past.  As I noted in the audit 
trail for 19199, there's stuff in build_modify_expr to handle 
MIN_EXPR/MAX_EXPR as lvalues -- but, if Roger wants to stick with that 
approach, it needs to be spread throughout the C++ front end.  I'd be 
happier with your approach overall, in part because I think fold is 
doing too much, too early, in general, but I don't feel comfortable 
approving the patch.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-04 Thread mmitchel at gcc dot gnu dot org

--- Additional Comments From mmitchel at gcc dot gnu dot org  2005-03-04 
08:13 ---
(Of course, the root cause of this problem is that fold is being called before
gimplification, which Nathan and I have sermonized about previously.)

There's interesting interplay between this PR and PR 20280.  In particular,
RTH's proposed transformation in Comment #8 is invalid if A and B are
bit-fields, because the address of a bitfield cannot be taken.

Consider:
  
  struct S { int i : 7; };
  S s;
  extern int b;
  const int a = (x ? s.i : b);

This cannot be transformed into:

  int *a = *((x ? t = s.i : t = *b), t);

as per Comment #8.

Indeed fold transforms ((s.i  i)? s.i : i) into MAX_EXPR (s.i, i) in this 
program:

struct S { int i : 7; };
S s;
extern int i;
bool b;
void g() {
  ((s.i  i) ? s.i : i) = 3;
}

That happens to work only because of the hack where we allow MAX_EXPR as an
lvalue in an assignment in build_modify_expr if neither argument has
side-effects.  (We recreate the COND_EXPR there.)  This change went in as the
fix for PR 7503.  

Roger, I think you need the same hack where you bind COND_EXPRs to references,
take their addresses, etc.  In fact, you should probably just make real_lvalue_p
accept MAX_EXPRs with side-effect free operands.  And, then, wherever we need
lvalues do the conversion that you presently do in build_modify_expr.  That's
consistent with your MAX_EXPRs are just the canonical form of certain
COND_EXPRs logic.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199


[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary

2005-03-04 Thread aoliva at redhat dot com

--- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-04 
19:08 ---
Subject: [PR c++/19199] don't turn cond_expr lvalue into min_expr rvalue
(continued from PR c++/20280)

On Mar  4, 2005, Mark Mitchell [EMAIL PROTECTED] wrote:

 Actually, looking at this more closely, I think that something is
 forgetting to call rationalize_conditional_expr, which is normally
 called from unary_complex_lvalue.  When the conditional expression is
 used in the lvalue context, something should be calling that.
 Normally, that happens because something calls build_unary_op
 (ADDR_EXPR, ...) on the COND_EXPR.

 It may be that I changed something to call build_addr (instead of
 build_unary_op) in a case where that's not safe.  Can you confirm or
 deny that hypothesis?

I'm not sure you're still talking about PR 20280, or about PR 19199,
that is marked as a blocker because it appears to be the same problem,
but AFAICT really isn't.

Here's a patch that fixes PR c++/19199, by avoiding the transformation
from:

cond lt nop(int) parm a  nop(int) parm b  
  parm a parm b 

into:

nop(enum) min nop(int) parm a  nop(int) parm b  

since the latter is clearly not an lvalue, and
rationalize_conditional_expr doesn't manage to turn it back into one
(I don't think it should).  I realize we might miss some optimization
opportunities because of this change in C, because the optimization
would be valid there, but I suppose we could arrange for cond_expr
operands in C to be forced into rvalues.

Still testing on x86_64-linux-gnu.  Ok to install if it passes?

Index: gcc/ChangeLog
from  Alexandre Oliva  [EMAIL PROTECTED]

PR c++/19199
* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_ternary): Use it to avoid turning a COND_EXPR lvalue into
an MIN_EXPR rvalue.

Index: gcc/fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.523
diff -u -p -r1.523 fold-const.c
--- gcc/fold-const.c 4 Mar 2005 06:24:09 - 1.523
+++ gcc/fold-const.c 4 Mar 2005 19:04:26 -
@@ -2004,16 +2004,12 @@ fold_convert (tree type, tree arg)
 }
 }
 
-/* Return an expr equal to X but certainly not valid as an lvalue.  */
+/* Return false if expr can be assumed to not be an lvalue, true
+   otherwise.  */
 
-tree
-non_lvalue (tree x)
+static bool
+maybe_lvalue_p (tree x)
 {
-  /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
- us.  */
-  if (in_gimple_form)
-return x;
-
   /* We only need to wrap lvalue tree codes.  */
   switch (TREE_CODE (x))
   {
@@ -2053,8 +2049,24 @@ non_lvalue (tree x)
 /* Assume the worst for front-end tree codes.  */
 if ((int)TREE_CODE (x) = NUM_TREE_CODES)
   break;
-return x;
+return false;
   }
+
+  return true;
+}
+
+/* Return an expr equal to X but certainly not valid as an lvalue.  */
+
+tree
+non_lvalue (tree x)
+{
+  /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
+ us.  */
+  if (in_gimple_form)
+return x;
+
+  if (! maybe_lvalue_p (x))
+return x;
   return build1 (NON_LVALUE_EXPR, TREE_TYPE (x), x);
 }
 
@@ -7095,10 +7107,17 @@ fold_ternary (tree expr)
 of B and C.  Signed zeros prevent all of these transformations,
 for reasons given above each one.
 
+We don't want to use operand_equal_for_comparison_p here,
+because it might turn an lvalue COND_EXPR into an rvalue one,
+see PR c++/19199.
+
  Also try swapping the arguments and inverting the conditional.  */
   if (COMPARISON_CLASS_P (arg0)
-  operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
-arg1, TREE_OPERAND (arg0, 1))
+  ((maybe_lvalue_p (arg1)
+   maybe_lvalue_p (TREE_OPERAND (t, 2)))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0), arg1, OEP_ONLY_CONST)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+   arg1, TREE_OPERAND (arg0, 1)))
   !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg1
{
  tem = fold_cond_expr_with_comparison (type, arg0,
@@ -7109,9 +7128,13 @@ fold_ternary (tree expr)
}
 
   if (COMPARISON_CLASS_P (arg0)
-  operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
-TREE_OPERAND (t, 2),
-TREE_OPERAND (arg0, 1))
+  ((maybe_lvalue_p (arg1)
+   maybe_lvalue_p (TREE_OPERAND (t, 2)))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0),
+TREE_OPERAND (t, 2), OEP_ONLY_CONST)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+   TREE_OPERAND (t, 2),
+   TREE_OPERAND (arg0, 1)))