diff mbox

RFC: C++ delayed folding merge

Message ID alpine.LSU.2.11.1511091001020.10078@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Nov. 9, 2015, 9:08 a.m. UTC
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


looks suspicious.  The issue here is that the vector elements will
have the wrong type after this simple handling.

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).  Not sure if we can even
write a testcase for such conversions with the GCC vector extensions
though.

Thanks,
Richard.

Comments

Jason Merrill Nov. 9, 2015, 7:28 p.m. UTC | #1
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.

> Not sure if we can even
> write a testcase for such conversions with the GCC vector extensions
> though.

Jason
diff mbox

Patch

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;
+       }
+    }