Patchwork [C++] Allow p->~T() when T is a vector

login
register
mail settings
Submitter Marc Glisse
Date Aug. 2, 2012, 11:42 a.m.
Message ID <alpine.DEB.2.02.1208021333001.23019@stedding.saclay.inria.fr>
Download mbox | patch
Permalink /patch/174737/
State New
Headers show

Comments

Marc Glisse - Aug. 2, 2012, 11:42 a.m.
Hello,

this patch allows p->~T() when T is (after substitution) a vector, which 
is necessary for use in std::vector for instance.

gcc/cp/ChangeLog
2012-08-02  Marc Glisse  <marc.glisse@inria.fr>

 	* pt.c (tsubst_copy_and_build): Handle VECTOR_TYPE like scalars.

gcc/testsuite/ChangeLog
2012-08-02  Marc Glisse  <marc.glisse@inria.fr>

 	* g++.dg/ext/vector17.C: New testcase.
Richard Guenther - Aug. 2, 2012, 12:08 p.m.
On Thu, Aug 2, 2012 at 1:42 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> this patch allows p->~T() when T is (after substitution) a vector, which is
> necessary for use in std::vector for instance.

Why not include VECTOR_TYPE in ARITHMETIC_TYPE_P?

> gcc/cp/ChangeLog
> 2012-08-02  Marc Glisse  <marc.glisse@inria.fr>
>
>         * pt.c (tsubst_copy_and_build): Handle VECTOR_TYPE like scalars.
>
> gcc/testsuite/ChangeLog
> 2012-08-02  Marc Glisse  <marc.glisse@inria.fr>
>
>         * g++.dg/ext/vector17.C: New testcase.
>
> --
> Marc Glisse
> Index: testsuite/g++.dg/ext/vector17.C
> ===================================================================
> --- testsuite/g++.dg/ext/vector17.C     (revision 0)
> +++ testsuite/g++.dg/ext/vector17.C     (revision 0)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +typedef double __attribute__((vector_size(1024) )) vec;
> +
> +template <class T>
> +void f (T *p)
> +{
> +  p->~T();
> +}
> +void g (vec *p)
> +{
> +  f(p);
> +}
>
> Property changes on: testsuite/g++.dg/ext/vector17.C
> ___________________________________________________________________
> Added: svn:eol-style
>    + native
> Added: svn:keywords
>    + Author Date Id Revision URL
>
> Index: cp/pt.c
> ===================================================================
> --- cp/pt.c     (revision 190071)
> +++ cp/pt.c     (working copy)
> @@ -13844,21 +13844,22 @@ tsubst_copy_and_build (tree t,
>                                     args, complain, in_decl);
>         else
>           member = tsubst_copy (member, args, complain, in_decl);
>         if (member == error_mark_node)
>           return error_mark_node;
>
>         if (type_dependent_expression_p (object))
>           /* We can't do much here.  */;
>         else if (!CLASS_TYPE_P (object_type))
>           {
> -           if (SCALAR_TYPE_P (object_type))
> +           if (SCALAR_TYPE_P (object_type)
> +               || TREE_CODE (object_type) == VECTOR_TYPE)
>               {
>                 tree s = NULL_TREE;
>                 tree dtor = member;
>
>                 if (TREE_CODE (dtor) == SCOPE_REF)
>                   {
>                     s = TREE_OPERAND (dtor, 0);
>                     dtor = TREE_OPERAND (dtor, 1);
>                   }
>                 if (TREE_CODE (dtor) == BIT_NOT_EXPR)
>
Ramana Radhakrishnan - Aug. 2, 2012, 12:15 p.m.
On 08/02/12 13:08, Richard Guenther wrote:
> On Thu, Aug 2, 2012 at 1:42 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this patch allows p->~T() when T is (after substitution) a vector, which is
>> necessary for use in std::vector for instance.
>
> Why not include VECTOR_TYPE in ARITHMETIC_TYPE_P?

Not really, because SCALAR_TYPE_P uses ARITHMETIC_TYPE_P just below in 
cp-tree.h to detect whether something is scalar or not. It would need 
more refactoring I'd suspect ....

Ramana

>
>> gcc/cp/ChangeLog
>> 2012-08-02  Marc Glisse  <marc.glisse@inria.fr>
>>
>>          * pt.c (tsubst_copy_and_build): Handle VECTOR_TYPE like scalars.
>>
>> gcc/testsuite/ChangeLog
>> 2012-08-02  Marc Glisse  <marc.glisse@inria.fr>
>>
>>          * g++.dg/ext/vector17.C: New testcase.
>>
>> --
>> Marc Glisse
>> Index: testsuite/g++.dg/ext/vector17.C
>> ===================================================================
>> --- testsuite/g++.dg/ext/vector17.C     (revision 0)
>> +++ testsuite/g++.dg/ext/vector17.C     (revision 0)
>> @@ -0,0 +1,12 @@
>> +/* { dg-do compile } */
>> +typedef double __attribute__((vector_size(1024) )) vec;
>> +
>> +template <class T>
>> +void f (T *p)
>> +{
>> +  p->~T();
>> +}
>> +void g (vec *p)
>> +{
>> +  f(p);
>> +}
>>
>> Property changes on: testsuite/g++.dg/ext/vector17.C
>> ___________________________________________________________________
>> Added: svn:eol-style
>>     + native
>> Added: svn:keywords
>>     + Author Date Id Revision URL
>>
>> Index: cp/pt.c
>> ===================================================================
>> --- cp/pt.c     (revision 190071)
>> +++ cp/pt.c     (working copy)
>> @@ -13844,21 +13844,22 @@ tsubst_copy_and_build (tree t,
>>                                      args, complain, in_decl);
>>          else
>>            member = tsubst_copy (member, args, complain, in_decl);
>>          if (member == error_mark_node)
>>            return error_mark_node;
>>
>>          if (type_dependent_expression_p (object))
>>            /* We can't do much here.  */;
>>          else if (!CLASS_TYPE_P (object_type))
>>            {
>> -           if (SCALAR_TYPE_P (object_type))
>> +           if (SCALAR_TYPE_P (object_type)
>> +               || TREE_CODE (object_type) == VECTOR_TYPE)
>>                {
>>                  tree s = NULL_TREE;
>>                  tree dtor = member;
>>
>>                  if (TREE_CODE (dtor) == SCOPE_REF)
>>                    {
>>                      s = TREE_OPERAND (dtor, 0);
>>                      dtor = TREE_OPERAND (dtor, 1);
>>                    }
>>                  if (TREE_CODE (dtor) == BIT_NOT_EXPR)
>>
>
Marc Glisse - Aug. 2, 2012, 12:20 p.m.
On Thu, 2 Aug 2012, Richard Guenther wrote:

> On Thu, Aug 2, 2012 at 1:42 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this patch allows p->~T() when T is (after substitution) a vector, which is
>> necessary for use in std::vector for instance.
>
> Why not include VECTOR_TYPE in ARITHMETIC_TYPE_P?

I often aim for the least disruptive change...

I just tried to look at the uses of ARITHMETIC_TYPE_P, and the first one I 
could find was to detect whether the type is convertible to bool :-(
Gabriel Dos Reis - Aug. 2, 2012, 2:19 p.m.
On Thu, Aug 2, 2012 at 7:08 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Aug 2, 2012 at 1:42 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> this patch allows p->~T() when T is (after substitution) a vector, which is
>> necessary for use in std::vector for instance.
>
> Why not include VECTOR_TYPE in ARITHMETIC_TYPE_P?

We have decided long ago that abstractions in the compiler
should use names and semantics following the standard texts
so that reading the source code should not reserve surprises
to anyone familiar with the standards, hence minimize help
subtle bugs and reduce efforts needed to understand the compiler.
Putting VECTOR_TYPE in ARITHMETIC_TYPE_P violates that
common sense principle.

-- Gaby
Jason Merrill - Aug. 2, 2012, 2:33 p.m.
On 08/02/2012 07:42 AM, Marc Glisse wrote:
> +	    if (SCALAR_TYPE_P (object_type)
> +		|| TREE_CODE (object_type) == VECTOR_TYPE)

You can use "scalarish_type_p" for this test.  OK with that change.

Jason
Marc Glisse - Aug. 2, 2012, 3:22 p.m.
On Thu, 2 Aug 2012, Jason Merrill wrote:

> On 08/02/2012 07:42 AM, Marc Glisse wrote:
>> +	    if (SCALAR_TYPE_P (object_type)
>> +		|| TREE_CODE (object_type) == VECTOR_TYPE)
>
> You can use "scalarish_type_p" for this test.  OK with that change.

Wow, I'd missed it, a function that does exactly what I need, thanks!

Patch

Index: testsuite/g++.dg/ext/vector17.C

===================================================================
--- testsuite/g++.dg/ext/vector17.C	(revision 0)

+++ testsuite/g++.dg/ext/vector17.C	(revision 0)

@@ -0,0 +1,12 @@ 

+/* { dg-do compile } */

+typedef double __attribute__((vector_size(1024) )) vec;

+

+template <class T>

+void f (T *p)

+{

+  p->~T();

+}

+void g (vec *p)

+{

+  f(p);

+}


Property changes on: testsuite/g++.dg/ext/vector17.C
___________________________________________________________________
Added: svn:eol-style
   + native
Added: svn:keywords
   + Author Date Id Revision URL

Index: cp/pt.c

===================================================================
--- cp/pt.c	(revision 190071)

+++ cp/pt.c	(working copy)

@@ -13844,21 +13844,22 @@  tsubst_copy_and_build (tree t,

 				    args, complain, in_decl);
 	else
 	  member = tsubst_copy (member, args, complain, in_decl);
 	if (member == error_mark_node)
 	  return error_mark_node;
 
 	if (type_dependent_expression_p (object))
 	  /* We can't do much here.  */;
 	else if (!CLASS_TYPE_P (object_type))
 	  {
-	    if (SCALAR_TYPE_P (object_type))

+	    if (SCALAR_TYPE_P (object_type)

+		|| TREE_CODE (object_type) == VECTOR_TYPE)

 	      {
 		tree s = NULL_TREE;
 		tree dtor = member;
 
 		if (TREE_CODE (dtor) == SCOPE_REF)
 		  {
 		    s = TREE_OPERAND (dtor, 0);
 		    dtor = TREE_OPERAND (dtor, 1);
 		  }
 		if (TREE_CODE (dtor) == BIT_NOT_EXPR)