Created attachment 8311 [details]
bzip2'ed dump

Confirmed. Reduced testcase:

==================================
struct A
{
    ~A();
};

struct B : A {};

A& foo();

void bar(bool b)
{
    (B&) (b ? foo() : foo());
}
==================================

Subject: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

When passing an lvalue cond_expr to a function taking a reference that
binds directly to either operand of ?:, we'd fail gimplification when
the type of the expressions was addressable, because, once again,
create_tmp_var would reject addressable types.

The solution I came up with was to hoist the indirect_ref out of the
cond_expr at the time it was created, such that we create a tmp_var
with a reference to the result of the cond_expr, and when we take the
address of the cond_expr to pass its result by reference, it cancels
out with the indirect_ref, so it works beautifully.

I went ahead and verified that I didn't break bit-field lvalues in
conditional expressions (my first attempt did), but I was surprised to
find out that the calls to h() pass.  I understand why they do (we
create a temporary and bind to that), but I'm not sure this is correct
behavior.  Opinions?

I'm bootstrapping this on x86_64-linux-gnu, along with the patch for
PR c++/20103; it's also passed C++ regression testing.  Ok to install
if bootstrap and all-languages regression testing passes?

Index: gcc/cp/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20280
	* call.c (build_conditional_expr): Hoist indirect_ref out of
	cond_expr if the result is an addressable lvalue.

Index: gcc/cp/call.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.531
diff -u -p -r1.531 call.c
--- gcc/cp/call.c 24 Feb 2005 21:55:10 -0000 1.531
+++ gcc/cp/call.c 3 Mar 2005 07:42:24 -0000
@@ -3111,6 +3111,7 @@ build_conditional_expr (tree arg1, tree 
   tree result = NULL_TREE;
   tree result_type = NULL_TREE;
   bool lvalue_p = true;
+  bool indirect_p = false;
   struct z_candidate *candidates = 0;
   struct z_candidate *cand;
   void *p;
@@ -3292,7 +3293,15 @@ build_conditional_expr (tree arg1, tree 
       && real_lvalue_p (arg3) 
       && same_type_p (arg2_type, arg3_type))
     {
-      result_type = arg2_type;
+      if (TREE_ADDRESSABLE (arg2_type) && TREE_ADDRESSABLE (arg3_type))
+	{
+	  indirect_p = true;
+	  result_type = build_pointer_type (arg2_type);
+	  arg2 = fold_if_not_in_template (build_address (arg2));
+	  arg3 = fold_if_not_in_template (build_address (arg3));
+	}
+      else
+	result_type = arg2_type;
       goto valid_operands;
     }
 
@@ -3458,6 +3467,14 @@ build_conditional_expr (tree arg1, tree 
   /* We can't use result_type below, as fold might have returned a
      throw_expr.  */
 
+  if (indirect_p)
+    {
+      if (TREE_CODE (TREE_TYPE (result)) == POINTER_TYPE)
+	result = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (result)), result);
+      else
+	gcc_assert (TREE_CODE (result) == THROW_EXPR);
+    }
+
   /* Expand both sides into the same slot, hopefully the target of the
      ?: expression.  We used to check for TARGET_EXPRs here, but now we
      sometimes wrap them in NOP_EXPRs so the test would fail.  */
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* g++.dg/tree-ssa/pr20103.C: New.

Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===================================================================
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 3 Mar 2005 07:42:38 -0000
@@ -0,0 +1,62 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+    ~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+    (B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <aoliva@redhat.com>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+  long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(const long&);
+
+void f(X &x, bool b)
+{
+  (b ? x.i : x.j) = 1;
+  (b ? x.j : x.k) = 2;
+  (b ? x.i : x.k) = 3;
+
+  (void)(b ? x.i : x.j);
+  (void)(b ? x.i : x.k);
+  (void)(b ? x.j : x.k);
+
+  g (b ? x.i : x.j); // { dg-error "cannot bind bitfield" }
+  g (b ? x.i : x.k); // { dg-error "cannot bind bitfield" }
+  g (b ? x.j : x.k); // { dg-error "cannot bind bitfield" }
+
+  // Hmm...  I don't think these should be accepted.  The conditional
+  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
+  // that are bit-fields, but not lvalues that are conditional
+  // expressions involving bit-fields.
+  h (b ? x.i : x.j);
+  h (b ? x.i : x.k);
+  h (b ? x.j : x.k);
+
+  (long &)(b ? x.i : x.j); // { dg-error "address of bit-field" }
+  (long &)(b ? x.i : x.k); // { dg-error "address of bit-field" }
+  (long &)(b ? x.j : x.k); // { dg-error "address of bit-field" }
+}

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

I think this is very much related to PR 19199.  I almost think when that bug is fixed this one will also be 
fixed.

Your current patch will not fix that PR either.

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs


On Mar 3, 2005, at 2:50 AM, Alexandre Oliva wrote:

> I'm bootstrapping this on x86_64-linux-gnu, along with the patch for
> PR c++/20103; it's also passed C++ regression testing.  Ok to install
> if bootstrap and all-languages regression testing passes?

I think this is the wrong approach, we should be doing the same for all
types (well except for bitfields) and not just "addressable" types, see
PR 19199 for a testcase where we get this wrong and there is a proposed
way of fixing this from RTH.

Thanks,
Andrew Pinski

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

Alexandre Oliva wrote:
\
> I went ahead and verified that I didn't break bit-field lvalues in
> conditional expressions (my first attempt did), but I was surprised to
> find out that the calls to h() pass.  I understand why they do (we
> create a temporary and bind to that), but I'm not sure this is correct
> behavior.  Opinions?

> +  // Hmm...  I don't think these should be accepted.  The conditional
> +  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
> +  // that are bit-fields, but not lvalues that are conditional
> +  // expressions involving bit-fields.
> +  h (b ? x.i : x.j);
> +  h (b ? x.i : x.k);
> +  h (b ? x.j : x.k);

That's legal because "h" takes a "const &", which permits the compiler 
to create a temporary.  If it takes a non-const reference, you should 
get an error.

And, I think these kinds of transformations (if necessary) should be 
done in a langhook during gimplification, not at COND_EXPR-creation 
time.  We really want the C++ front-end's data structures to be an 
accurate mirror of the input program for as long as possible.

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Mar  3, 2005, Mark Mitchell <mark@codesourcery.com> wrote:

> Alexandre Oliva wrote:
> \
>> I went ahead and verified that I didn't break bit-field lvalues in
>> conditional expressions (my first attempt did), but I was surprised to
>> find out that the calls to h() pass.  I understand why they do (we
>> create a temporary and bind to that), but I'm not sure this is correct
>> behavior.  Opinions?

>> +  // Hmm...  I don't think these should be accepted.  The conditional
>> +  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
>> +  // that are bit-fields, but not lvalues that are conditional
>> +  // expressions involving bit-fields.
>> +  h (b ? x.i : x.j);
>> +  h (b ? x.i : x.k);
>> +  h (b ? x.j : x.k);

> That's legal because "h" takes a "const &", which permits the compiler
> to create a temporary.

Yeah, it permits, but only in certain circumstances that AFAICT aren't
met.  This expression AFAICT is an lvalue that isn't a bit-field, so
it has to bind directly, per the first bullet in 8.5.3/5.  Since it
meets the conditions of this first bullet, it doesn't get to use the
`otherwise' portion of that paragraph, that creates a temporary.  Or
am I misreading anything?

> And, I think these kinds of transformations (if necessary) should be
> done in a langhook during gimplification, not at COND_EXPR-creation
> time.  We really want the C++ front-end's data structures to be an
> accurate mirror of the input program for as long as possible.

Err...  But in what sense does my patch change that?  See, what I'm
doing is hoisting the indirect_refs that are inserted by
stabilize_reference out of the cond_expr.  They weren't in the
original code.  There's no dereferencing going on unless the whole
expression undergoes lvalue-to-rvalue decay, so I'd argue that the
transformation I'm proposing actually matches even more accurately the
meaning of the original source code.

(In reply to comment #7)
> Yeah, it permits, but only in certain circumstances that AFAICT aren't
> met.  This expression AFAICT is an lvalue that isn't a bit-field, so
> it has to bind directly, per the first bullet in 8.5.3/5.  Since it
> meets the conditions of this first bullet, it doesn't get to use the
> `otherwise' portion of that paragraph, that creates a temporary.  Or
> am I misreading anything?
Yes these expressions are lvalues, see again PR 19199 where we get this wrong.  Even Mark and RTH 
commented on it.  Again it looks like the C++ parser should be creating a tempary to store the address 
and have the indirect reference out of the COND_EXPR, this can only happen for non bitfields.

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Mar  3, 2005, Andrew Pinski <pinskia@physics.uc.edu> wrote:

> On Mar 3, 2005, at 2:50 AM, Alexandre Oliva wrote:

>> I'm bootstrapping this on x86_64-linux-gnu, along with the patch for
>> PR c++/20103; it's also passed C++ regression testing.  Ok to install
>> if bootstrap and all-languages regression testing passes?

> I think this is the wrong approach,

Err...  But AFAICT this is exactly the approach RTH suggested to cope
with the issue, except for the removal of the unnecessary artificial
decl before gimplification.

> we should be doing the same for all types (well except for
> bitfields) and not just "addressable" types,

Agreed.  That's relatively easy to fix.

Improved patch follows.  Ok to install?

Index: gcc/cp/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/19199
	PR c++/20280
	* call.c (build_conditional_expr): Hoist indirect_ref out of
	cond_expr if the result is a non-bitfield lvalue.

Index: gcc/cp/call.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.531
diff -u -p -r1.531 call.c
--- gcc/cp/call.c 24 Feb 2005 21:55:10 -0000 1.531
+++ gcc/cp/call.c 4 Mar 2005 06:32:44 -0000
@@ -3111,6 +3111,9 @@ build_conditional_expr (tree arg1, tree 
   tree result = NULL_TREE;
   tree result_type = NULL_TREE;
   bool lvalue_p = true;
+  bool indirect_p = false;
+  cp_lvalue_kind arg2_lvalue_kind;
+  cp_lvalue_kind arg3_lvalue_kind;
   struct z_candidate *candidates = 0;
   struct z_candidate *cand;
   void *p;
@@ -3288,11 +3291,26 @@ build_conditional_expr (tree arg1, tree 
 
      If the second and third operands are lvalues and have the same
      type, the result is of that type and is an lvalue.  */
-  if (real_lvalue_p (arg2) 
-      && real_lvalue_p (arg3) 
+  if ((arg2_lvalue_kind = real_lvalue_p (arg2))
+      && (arg3_lvalue_kind = real_lvalue_p (arg3))
       && same_type_p (arg2_type, arg3_type))
     {
-      result_type = arg2_type;
+      if ((arg2_lvalue_kind & clk_bitfield) == clk_none
+	  && (arg3_lvalue_kind & clk_bitfield) == clk_none)
+	{
+	  indirect_p = true;
+	  result_type = build_pointer_type (arg2_type);
+	  if (TREE_CODE (arg2) == INDIRECT_REF)
+	    arg2 = TREE_OPERAND (arg2, 0);
+	  else
+	    arg2 = fold_if_not_in_template (build_address (arg2));
+	  if (TREE_CODE (arg3) == INDIRECT_REF)
+	    arg3 = TREE_OPERAND (arg3, 0);
+	  else
+	    arg3 = fold_if_not_in_template (build_address (arg3));
+	}
+      else
+	result_type = arg2_type;
       goto valid_operands;
     }
 
@@ -3458,6 +3476,14 @@ build_conditional_expr (tree arg1, tree 
   /* We can't use result_type below, as fold might have returned a
      throw_expr.  */
 
+  if (indirect_p)
+    {
+      if (TREE_CODE (TREE_TYPE (result)) == POINTER_TYPE)
+	result = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (result)), result);
+      else
+	gcc_assert (TREE_CODE (result) == THROW_EXPR);
+    }
+
   /* Expand both sides into the same slot, hopefully the target of the
      ?: expression.  We used to check for TARGET_EXPRs here, but now we
      sometimes wrap them in NOP_EXPRs so the test would fail.  */
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/19199
	PR c++/20280
	* g++.dg/tree-ssa/pr20280.C: New.

Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===================================================================
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 4 Mar 2005 06:32:58 -0000
@@ -0,0 +1,80 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+    ~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+    (B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <aoliva@redhat.com>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+  long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(const long&);
+
+void f(X &x, bool b)
+{
+  (b ? x.i : x.j) = 1;
+  (b ? x.j : x.k) = 2;
+  (b ? x.i : x.k) = 3;
+
+  (void)(b ? x.i : x.j);
+  (void)(b ? x.i : x.k);
+  (void)(b ? x.j : x.k);
+
+  g (b ? x.i : x.j); // { dg-error "cannot bind bitfield" }
+  g (b ? x.i : x.k); // { dg-error "cannot bind bitfield" }
+  g (b ? x.j : x.k); // { dg-error "cannot bind bitfield" }
+
+  // Hmm...  I don't think these should be accepted.  The conditional
+  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
+  // that are bit-fields, but not lvalues that are conditional
+  // expressions involving bit-fields.
+  h (b ? x.i : x.j);
+  h (b ? x.i : x.k);
+  h (b ? x.j : x.k);
+
+  (long &)(b ? x.i : x.j); // { dg-error "address of bit-field" }
+  (long &)(b ? x.i : x.k); // { dg-error "address of bit-field" }
+  (long &)(b ? x.j : x.k); // { dg-error "address of bit-field" }
+}
+
+// What follows is from PR c++/19199, yet another bug on cond-expr
+// lvalues.  We used to return a reference to a temporary in qMin.
+
+enum Foo { A, B };
+
+template<typename T> const T &qMin(const T &a, const T &b) 
+{
+  return a < b ? a : b;
+}
+
+int testref (int,  char **)
+{
+  Foo f = A;
+  Foo g = B;
+  Foo h = qMin(f, g);
+  return 0;
+}

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Mar  4, 2005, Alexandre Oliva <aoliva@redhat.com> wrote:

>> we should be doing the same for all types (well except for
>> bitfields) and not just "addressable" types,

> Agreed.  That's relatively easy to fix.

Rats.  Not that easy.  A number of regressions showed up with the
`improved' patch :-(

It has to do with the uses of build_address, that marks variables and
fields as addressable and used, so we end up having to emit them,
instead of optimizing them out as intended.

It seems like we may indeed need something more elaborate at
gimplification time, instead of modifying the up-front representation.

I'll keep digging.

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

Alexandre Oliva wrote:

> 
>>>+  // Hmm...  I don't think these should be accepted.  The conditional
>>>+  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
>>>+  // that are bit-fields, but not lvalues that are conditional
>>>+  // expressions involving bit-fields.
>>>+  h (b ? x.i : x.j);
>>>+  h (b ? x.i : x.k);
>>>+  h (b ? x.j : x.k);
> 
> 
>>That's legal because "h" takes a "const &", which permits the compiler
>>to create a temporary.
> 
> 
> Yeah, it permits, but only in certain circumstances that AFAICT aren't
> met.  This expression AFAICT is an lvalue that isn't a bit-field, so
> it has to bind directly, per the first bullet in 8.5.3/5.  Since it
> meets the conditions of this first bullet, it doesn't get to use the
> `otherwise' portion of that paragraph, that creates a temporary.  Or
> am I misreading anything?

The situation is a little unclear.

EDG also accepts this code, which is part of what confused me.

Your reading is logical, but it depends on exactly what "lvalue for a 
bit-field" means.  (Note that it does not say "lvalue *is* a bit-field"; 
it says "lvalue *for* a bit-field".)

Consider:

   h((0, x.i));

It would be odd not to allow that, but to allow "h(x.i)".  The 
comma-expression isn't changing what's being passed to "h".  The same 
goes for "h((x.i = 3))".

I think that, if anything, there's a possible defect in the standard 
here, not a defect in the compiler.

>>And, I think these kinds of transformations (if necessary) should be
>>done in a langhook during gimplification, not at COND_EXPR-creation
>>time.  We really want the C++ front-end's data structures to be an
>>accurate mirror of the input program for as long as possible.
>  
> Err...  But in what sense does my patch change that?  See, what I'm
> doing is hoisting the indirect_refs that are inserted by
> stabilize_reference out of the cond_expr.  They weren't in the
> original code.  There's no dereferencing going on unless the whole
> expression undergoes lvalue-to-rvalue decay, so I'd argue that the
> transformation I'm proposing actually matches even more accurately the
> meaning of the original source code.

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?

Thanks,

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Mar  4, 2005, Mark Mitchell <mark@codesourcery.com> wrote:

> Your reading is logical, but it depends on exactly what "lvalue for a
> bit-field" means.  (Note that it does not say "lvalue *is* a
> bit-field"; it says "lvalue *for* a bit-field".)

Good point.  Here's an all-new patch, with the comment updated to
reflect our discussion.

Still testing on x86_64-linux-gnu, ok to install if it succeeds?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20280
	* gimplify.c (gimplify_cond_expr): Add fallback argument.  Use a
	temporary variable of pointer type if an lvalues is required.
	(gimplify_modify_expr_rhs): Request an rvalue from it.
	(gimplify_expr): Pass fallback on.

Index: gcc/gimplify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.113
diff -u -p -r2.113 gimplify.c
--- gcc/gimplify.c 18 Feb 2005 19:35:37 -0000 2.113
+++ gcc/gimplify.c 4 Mar 2005 19:18:49 -0000
@@ -2123,7 +2123,8 @@ gimple_boolify (tree expr)
      *EXPR_P should be stored.  */
 
 static enum gimplify_status
-gimplify_cond_expr (tree *expr_p, tree *pre_p, tree *post_p, tree target)
+gimplify_cond_expr (tree *expr_p, tree *pre_p, tree *post_p, tree target,
+		    fallback_t fallback)
 {
   tree expr = *expr_p;
   tree tmp, tmp2, type;
@@ -2137,18 +2138,40 @@ gimplify_cond_expr (tree *expr_p, tree *
      the arms.  */
   else if (! VOID_TYPE_P (type))
     {
+      tree result;
+
       if (target)
 	{
 	  ret = gimplify_expr (&target, pre_p, post_p,
 			       is_gimple_min_lval, fb_lvalue);
 	  if (ret != GS_ERROR)
 	    ret = GS_OK;
-	  tmp = target;
+	  result = tmp = target;
 	  tmp2 = unshare_expr (target);
 	}
+      else if ((fallback & fb_lvalue) == 0)
+	{
+	  result = tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
+	  ret = GS_ALL_DONE;
+	}
       else
 	{
-	  tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
+	  tree type = build_pointer_type (TREE_TYPE (expr));
+
+	  if (TREE_TYPE (TREE_OPERAND (expr, 1)) != void_type_node)
+	    TREE_OPERAND (expr, 1) =
+	      build_fold_addr_expr (TREE_OPERAND (expr, 1));
+
+	  if (TREE_TYPE (TREE_OPERAND (expr, 2)) != void_type_node)
+	    TREE_OPERAND (expr, 2) =
+	      build_fold_addr_expr (TREE_OPERAND (expr, 2));
+	  
+	  tmp2 = tmp = create_tmp_var (type, "iftmp");
+
+	  expr = build (COND_EXPR, void_type_node, TREE_OPERAND (expr, 0),
+			TREE_OPERAND (expr, 1), TREE_OPERAND (expr, 2));
+
+	  result = build_fold_indirect_ref (tmp);
 	  ret = GS_ALL_DONE;
 	}
 
@@ -2169,7 +2192,7 @@ gimplify_cond_expr (tree *expr_p, tree *
       /* Move the COND_EXPR to the prequeue.  */
       gimplify_and_add (expr, pre_p);
 
-      *expr_p = tmp;
+      *expr_p = result;
       return ret;
     }
 
@@ -2907,7 +2930,8 @@ gimplify_modify_expr_rhs (tree *expr_p, 
 	if (!is_gimple_reg_type (TREE_TYPE (*from_p)))
 	  {
 	    *expr_p = *from_p;
-	    return gimplify_cond_expr (expr_p, pre_p, post_p, *to_p);
+	    return gimplify_cond_expr (expr_p, pre_p, post_p, *to_p,
+				       fb_rvalue);
 	  }
 	else
 	  ret = GS_UNHANDLED;
@@ -3721,7 +3745,8 @@ gimplify_expr (tree *expr_p, tree *pre_p
 	  break;
 
 	case COND_EXPR:
-	  ret = gimplify_cond_expr (expr_p, pre_p, post_p, NULL_TREE);
+	  ret = gimplify_cond_expr (expr_p, pre_p, post_p, NULL_TREE,
+				    fallback);
 	  break;
 
 	case CALL_EXPR:
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20280
	* g++.dg/tree-ssa/pr20280.C: New.

Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===================================================================
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 4 Mar 2005 19:19:03 -0000
@@ -0,0 +1,63 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+    ~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+    (B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <aoliva@redhat.com>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+  long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(const long&);
+
+void f(X &x, bool b)
+{
+  (b ? x.i : x.j) = 1;
+  (b ? x.j : x.k) = 2;
+  (b ? x.i : x.k) = 3;
+
+  (void)(b ? x.i : x.j);
+  (void)(b ? x.i : x.k);
+  (void)(b ? x.j : x.k);
+
+  g (b ? x.i : x.j); // { dg-error "cannot bind bitfield" }
+  g (b ? x.i : x.k); // { dg-error "cannot bind bitfield" }
+  g (b ? x.j : x.k); // { dg-error "cannot bind bitfield" }
+
+  // It's not entirely clear whether these should be accepted.  The
+  // conditional expressions are lvalues for sure, and 8.5.3/5 exempts
+  // lvalues for bit-fields, but it's not clear that conditional
+  // expressions that are lvalues and that have at least one possible
+  // result that is a bit-field lvalue meets this condition.
+  h (b ? x.i : x.j);
+  h (b ? x.i : x.k);
+  h (b ? x.j : x.k);
+
+  (long &)(b ? x.i : x.j); // { dg-error "address of bit-field" }
+  (long &)(b ? x.i : x.k); // { dg-error "address of bit-field" }
+  (long &)(b ? x.j : x.k); // { dg-error "address of bit-field" }
+}

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Thu, 03 Mar 2005 23:26:04 -0800, Mark Mitchell <mark@codesourcery.com> wrote:

> Your reading is logical, but it depends on exactly what "lvalue for a
> bit-field" means.  (Note that it does not say "lvalue *is* a bit-field"; it
> says "lvalue *for* a bit-field".)

In fact, there's a core issue for exactly that question; Steve's proposed
wording clarifies that these expressions are lvalues for a bit-field.

Jason

Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Fri, Mar 04, 2005 at 04:21:53PM -0300, Alexandre Oliva wrote:
> 	* gimplify.c (gimplify_cond_expr): Add fallback argument.  Use a
> 	temporary variable of pointer type if an lvalues is required.
> 	(gimplify_modify_expr_rhs): Request an rvalue from it.
> 	(gimplify_expr): Pass fallback on.

Ok.


r~

Subject: Bug 20280

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	aoliva@gcc.gnu.org	2005-03-14 20:02:11

Modified files:
	gcc            : ChangeLog gimplify.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/tree-ssa: pr20280.C 

Log message:
	gcc/ChangeLog:
	PR c++/20280
	* gimplify.c (gimplify_cond_expr): Add fallback argument.  Use a
	temporary variable of pointer type if an lvalues is required.
	(gimplify_modify_expr_rhs): Request an rvalue from it.
	(gimplify_expr): Pass fallback on.
	gcc/testsuite/ChangeLog:
	PR c++/20280
	* g++.dg/tree-ssa/pr20280.C: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.7848&r2=2.7849
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/gimplify.c.diff?cvsroot=gcc&r1=2.117&r2=2.118
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.5165&r2=1.5166
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/tree-ssa/pr20280.C.diff?cvsroot=gcc&r1=NONE&r2=1.1

Subject: Bug 20280

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-4_0-branch
Changes by:	aoliva@gcc.gnu.org	2005-03-14 20:34:53

Modified files:
	gcc            : ChangeLog gimplify.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/tree-ssa: pr20280.C 

Log message:
	gcc/ChangeLog:
	PR c++/20280
	* gimplify.c (gimplify_cond_expr): Add fallback argument.  Use a
	temporary variable of pointer type if an lvalues is required.
	(gimplify_modify_expr_rhs): Request an rvalue from it.
	(gimplify_expr): Pass fallback on.
	gcc/testsuite/ChangeLog:
	PR c++/20280
	* g++.dg/tree-ssa/pr20280.C: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.7592.2.50&r2=2.7592.2.51
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/gimplify.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.113.2.1&r2=2.113.2.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.5084.2.45&r2=1.5084.2.46
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/tree-ssa/pr20280.C.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.2.1

Fixed

The gimplifier patch here causes PR 30132 and I still think it is the wrong approach as we have a fall back of either here.  If I change "fallback & fb_lvalue) == 0" to "fallback != fb_lvalue", then we get an ICE still on this testcase but since the fallback says it can be either.

I have a patch which fixes after my patch for PR 30132.  The way I fixed it is an all front-end fix:
Index: typeck.c
===================================================================
--- typeck.c    (revision 122880)
+++ typeck.c    (working copy)
@@ -4060,6 +4060,14 @@ build_address (tree t)
   if (error_operand_p (t) || !cxx_mark_addressable (t))
     return error_mark_node;

+  if (TREE_CODE (t) == COND_EXPR)
+    {
+      tree ptrtype = build_pointer_type (TREE_TYPE (t));
+      return build3 (COND_EXPR, ptrtype, TREE_OPERAND (t, 0),
+                    build_address (TREE_OPERAND (t, 1)),
+                    build_address (TREE_OPERAND (t, 2)));
+    }
+
   addr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t);

   return addr;


Create a new COND_EXPR for when we want to create an address of a COND_EXPR, by this point, we should have errored out.