diff mbox

RFC: C++ delayed folding merge

Message ID 56410851.2080502@redhat.com
State New
Headers show

Commit Message

Jason Merrill Nov. 9, 2015, 8:55 p.m. UTC
On 11/09/2015 02:28 PM, Jason Merrill wrote:
> On 11/09/2015 04:08 AM, Richard Biener wrote:
>> On Mon, 9 Nov 2015, Jason Merrill wrote:
>>
>>> I'm planning to merge the C++ delayed folding branch this week, but I
>>> need to
>>> get approval of the back end changes (the first patch attached).
>>> Most of
>>> these are the introduction of non-folding variants of convert_to_*,
>>> but there
>>> are a few others.
>>>
>>> One question: The branch changes 'convert' to not fold its result,
>>> and it's
>>> not clear to me whether that's part of the expected behavior of a
>>> front end
>>> 'convert' function or not.
>>
>> History.  convert is purely frontend (but shared, unfortunately between
>> all frontends).  I would expect that FEs that do not do delayed folding
>> expect convert to fold.
>>
>>> Also, I'm a bit uncertain about merging this at the end of stage 1,
>>> since it's
>>> a large internal change with relatively small user impact; it just
>>> improves
>>> handling of constant expression corner cases.  I'm inclined to go
>>> ahead with
>>> it at this point, but I'm interested in contrary opinions.
>>
>> I welcome this change as it should allow cleaning up the FE-middle-end
>> interface a bit more.  It should be possible to remove all
>> NON_LVALUE_EXPR adding/removal from the middle-end folders.
>>
>> Looks like the backend patch included frontend parts but as far as I
>> skimmed it only
>>
>> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
>> index 5e32901..d754a90 100644
>> --- a/gcc/fold-const.c
>> +++ b/gcc/fold-const.c
>> @@ -2091,6 +2091,17 @@ fold_convert_const (enum tree_code code, tree
>> type,
>> tree arg1)
>>         else if (TREE_CODE (arg1) == REAL_CST)
>>          return fold_convert_const_fixed_from_real (type, arg1);
>>       }
>> +  else if (TREE_CODE (type) == VECTOR_TYPE)
>> +    {
>> +      if (TREE_CODE (arg1) == VECTOR_CST
>> +         && TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (TREE_TYPE
>> (arg1))
>> +         && TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
>> +       {
>> +         tree r = copy_node (arg1);
>> +         TREE_TYPE (arg1) = type;
>> +         return r;
>> +       }
>> +    }
>>
>>
>> looks suspicious.  The issue here is that the vector elements will
>> have the wrong type after this simple handling.
>
> I was aiming to just handle simple cv-qualifier changes; that's why the
> TYPE_MAIN_VARIANT comparison is there.
>
>> If you fix that you can as well handle all kind of element type
>> changes via recursing to fold_convert_const (that includes
>> float to int / int to float changes).
>
> But I'll try this.

Like so?

Jason

Comments

Richard Biener Nov. 10, 2015, 9:02 a.m. UTC | #1
On Mon, 9 Nov 2015, Jason Merrill wrote:

> On 11/09/2015 02:28 PM, Jason Merrill wrote:
> > On 11/09/2015 04:08 AM, Richard Biener wrote:
> > > On Mon, 9 Nov 2015, Jason Merrill wrote:
> > > 
> > > > I'm planning to merge the C++ delayed folding branch this week, but I
> > > > need to
> > > > get approval of the back end changes (the first patch attached).
> > > > Most of
> > > > these are the introduction of non-folding variants of convert_to_*,
> > > > but there
> > > > are a few others.
> > > > 
> > > > One question: The branch changes 'convert' to not fold its result,
> > > > and it's
> > > > not clear to me whether that's part of the expected behavior of a
> > > > front end
> > > > 'convert' function or not.
> > > 
> > > History.  convert is purely frontend (but shared, unfortunately between
> > > all frontends).  I would expect that FEs that do not do delayed folding
> > > expect convert to fold.
> > > 
> > > > Also, I'm a bit uncertain about merging this at the end of stage 1,
> > > > since it's
> > > > a large internal change with relatively small user impact; it just
> > > > improves
> > > > handling of constant expression corner cases.  I'm inclined to go
> > > > ahead with
> > > > it at this point, but I'm interested in contrary opinions.
> > > 
> > > I welcome this change as it should allow cleaning up the FE-middle-end
> > > interface a bit more.  It should be possible to remove all
> > > NON_LVALUE_EXPR adding/removal from the middle-end folders.
> > > 
> > > Looks like the backend patch included frontend parts but as far as I
> > > skimmed it only
> > > 
> > > diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> > > index 5e32901..d754a90 100644
> > > --- a/gcc/fold-const.c
> > > +++ b/gcc/fold-const.c
> > > @@ -2091,6 +2091,17 @@ fold_convert_const (enum tree_code code, tree
> > > type,
> > > tree arg1)
> > >         else if (TREE_CODE (arg1) == REAL_CST)
> > >          return fold_convert_const_fixed_from_real (type, arg1);
> > >       }
> > > +  else if (TREE_CODE (type) == VECTOR_TYPE)
> > > +    {
> > > +      if (TREE_CODE (arg1) == VECTOR_CST
> > > +         && TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (TREE_TYPE
> > > (arg1))
> > > +         && TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
> > > +       {
> > > +         tree r = copy_node (arg1);
> > > +         TREE_TYPE (arg1) = type;
> > > +         return r;
> > > +       }
> > > +    }
> > > 
> > > 
> > > looks suspicious.  The issue here is that the vector elements will
> > > have the wrong type after this simple handling.
> > 
> > I was aiming to just handle simple cv-qualifier changes; that's why the
> > TYPE_MAIN_VARIANT comparison is there.
> > 
> > > If you fix that you can as well handle all kind of element type
> > > changes via recursing to fold_convert_const (that includes
> > > float to int / int to float changes).
> > 
> > But I'll try this.
> 
> Like so?

Yes.

Thanks,
Richard.
diff mbox

Patch

diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index ae16cfc..d8c7faf 100644
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index f9e5064..927e623 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -2095,6 +2095,25 @@  fold_convert_const (enum tree_code code, tree type, tree arg1)
       else if (TREE_CODE (arg1) == REAL_CST)
 	return fold_convert_const_fixed_from_real (type, arg1);
     }
+  else if (TREE_CODE (type) == VECTOR_TYPE)
+    {
+      if (TREE_CODE (arg1) == VECTOR_CST
+	  && TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
+	{
+	  int len = TYPE_VECTOR_SUBPARTS (type);
+	  tree elttype = TREE_TYPE (type);
+	  tree *v = XALLOCAVEC (tree, len);
+	  for (int i = 0; i < len; ++i)
+	    {
+	      tree elt = VECTOR_CST_ELT (arg1, i);
+	      tree cvt = fold_convert_const (code, elttype, elt);
+	      if (cvt == NULL_TREE)
+		return NULL_TREE;
+	      v[i] = cvt;
+	    }
+	  return build_vector (type, v);
+	}
+    }
   return NULL_TREE;
 }