diff mbox

Replace C/C++ void_zero_node with a VOID_CST tree code

Message ID 87lhtygk5x.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com
State New
Headers show

Commit Message

Richard Sandiford May 19, 2014, 10:03 a.m. UTC
Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, May 19, 2014 at 11:27 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Sat, May 17, 2014 at 10:15 AM, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>>> The main thing keeping zero-precision wide-ints alive was void_zero_node,
>>>> a tree used in the C and C++ frontends that has type VOID_TYPE but code
>>>> INTEGER_CST.
>>>>
>>>> Richard B. asked me to replace the INTEGER_CST with a new constant type,
>>>> here called VOID_CST.  Most of it is straight-forward.  The one perhaps
>>>> controversial bit is that C++ uses void_(zero_)node to represent dummy
>>>> objects when e.g. taking the address of a member function without an
>>>> associated object.  IIUC the node used in this situation needs to be
>>>> distinct from anything that could occur in user code and therefore couldn't
>>>> be a simple null pointer.
>>>>
>>>> This reaches the gimplifier in cases like
>>>> g++.old-deja/g++.brendan/operators4.C.  I chose to handle it in the
>>>> gimplifier, since void_zero_node was previously handled there too,
>>>> although perhaps by accident.  If you object strongly to this then
>>>> I'll need pretty detailed instructions about what to do instead,
>>>> i.e. exactly which parts of the C++ front end need to be changed
>>>> in order for dummy objects never to escape.
>>>
>>> I suppose it reaches the gimplifier because it's not handled in
>>> fold-const.c:fold_convert_loc while the INTEGER_CST void_zero_node
>>> was (through fold_convert_const).
>>
>> But like I said, void_zero_node reached the gimplifier too.  Try adding:
>>
>>   gcc_assert (TREE_TYPE (TREE_OPERAND (*expr_p, 0)) != void_type_node
>>               || TREE_CODE (TREE_OPERAND (*expr_p, 0)) != INTEGER_CST);
>>
>> to gimplify_conversion and running g++.old-deja/g++.brendan/operators4.C
>> to see what I mean.
>>
>>> That said, only handling (T)void_cst in gimplification looks like
>>> a hack.  If necessary we'd want to treat it as construct-T-with-zero-value
>>> consistently.
>>
>> OK, so just remove the gcc_checking_assert?
>
> Which one?

The one I'd added to the gimplifier:

> I'd add VOID_CST handling to fold_convert_const.

But like you say, if that was enough, void_zero_node would never
have reached the gimplifier, whereas as above it does.  I tried adding:

  gcc_assert (TREE_CODE (arg1) != VOID_CST);

to fold_convert_const and it doesn't trigger for operators4.C.
It also doesn't trigger for g++-mike/p10769b.C, which was the
other main motivating case.

Thanks,
Richard

Comments

Richard Biener May 19, 2014, 12:21 p.m. UTC | #1
On Mon, May 19, 2014 at 12:03 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, May 19, 2014 at 11:27 AM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>> On Sat, May 17, 2014 at 10:15 AM, Richard Sandiford
>>>> <rdsandiford@googlemail.com> wrote:
>>>>> The main thing keeping zero-precision wide-ints alive was void_zero_node,
>>>>> a tree used in the C and C++ frontends that has type VOID_TYPE but code
>>>>> INTEGER_CST.
>>>>>
>>>>> Richard B. asked me to replace the INTEGER_CST with a new constant type,
>>>>> here called VOID_CST.  Most of it is straight-forward.  The one perhaps
>>>>> controversial bit is that C++ uses void_(zero_)node to represent dummy
>>>>> objects when e.g. taking the address of a member function without an
>>>>> associated object.  IIUC the node used in this situation needs to be
>>>>> distinct from anything that could occur in user code and therefore couldn't
>>>>> be a simple null pointer.
>>>>>
>>>>> This reaches the gimplifier in cases like
>>>>> g++.old-deja/g++.brendan/operators4.C.  I chose to handle it in the
>>>>> gimplifier, since void_zero_node was previously handled there too,
>>>>> although perhaps by accident.  If you object strongly to this then
>>>>> I'll need pretty detailed instructions about what to do instead,
>>>>> i.e. exactly which parts of the C++ front end need to be changed
>>>>> in order for dummy objects never to escape.
>>>>
>>>> I suppose it reaches the gimplifier because it's not handled in
>>>> fold-const.c:fold_convert_loc while the INTEGER_CST void_zero_node
>>>> was (through fold_convert_const).
>>>
>>> But like I said, void_zero_node reached the gimplifier too.  Try adding:
>>>
>>>   gcc_assert (TREE_TYPE (TREE_OPERAND (*expr_p, 0)) != void_type_node
>>>               || TREE_CODE (TREE_OPERAND (*expr_p, 0)) != INTEGER_CST);
>>>
>>> to gimplify_conversion and running g++.old-deja/g++.brendan/operators4.C
>>> to see what I mean.
>>>
>>>> That said, only handling (T)void_cst in gimplification looks like
>>>> a hack.  If necessary we'd want to treat it as construct-T-with-zero-value
>>>> consistently.
>>>
>>> OK, so just remove the gcc_checking_assert?
>>
>> Which one?
>
> The one I'd added to the gimplifier:
>
> Index: gcc/gimplify.c
> ===================================================================
> --- gcc/gimplify.c      2014-05-15 13:49:24.483656013 +0100
> +++ gcc/gimplify.c      2014-05-16 17:55:17.087700837 +0100
> @@ -1681,7 +1681,15 @@ gimplify_conversion (tree *expr_p)
>    /* Then strip away all but the outermost conversion.  */
>    STRIP_SIGN_NOPS (TREE_OPERAND (*expr_p, 0));
>
> -  /* And remove the outermost conversion if it's useless.  */
> +  /* Support C++-style dummy objects, in which void_zero is
> +     cast to a pointer type.  We treat these as null pointers.  */
> +  if (TREE_OPERAND (*expr_p, 0) == void_node)
> +    {
> +      gcc_checking_assert (POINTER_TYPE_P (TREE_TYPE (*expr_p)));
> +      *expr_p = build_int_cst (TREE_TYPE (*expr_p), 0);
> +    }
> +
> +  /* Remove the outermost conversion if it's useless.  */
>    if (tree_ssa_useless_type_conversion (*expr_p))
>      *expr_p = TREE_OPERAND (*expr_p, 0);

Ah.  yes, remove the assert and instead use

  *expr_p = build_zero_cost (TREE_TYPE (*expr_p));

>> I'd add VOID_CST handling to fold_convert_const.
>
> But like you say, if that was enough, void_zero_node would never
> have reached the gimplifier, whereas as above it does.  I tried adding:
>
>   gcc_assert (TREE_CODE (arg1) != VOID_CST);
>
> to fold_convert_const and it doesn't trigger for operators4.C.
> It also doesn't trigger for g++-mike/p10769b.C, which was the
> other main motivating case.

Ok, I see.  Thus fine IMHO with doing the above (though I wonder
why we couldn't have done this in the FE specific gimplification
routines for void_zero_node ...?)

Still defer to C++ people for the C++ parts.

Thanks,
Richard.

> Thanks,
> Richard
diff mbox

Patch

Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	2014-05-15 13:49:24.483656013 +0100
+++ gcc/gimplify.c	2014-05-16 17:55:17.087700837 +0100
@@ -1681,7 +1681,15 @@  gimplify_conversion (tree *expr_p)
   /* Then strip away all but the outermost conversion.  */
   STRIP_SIGN_NOPS (TREE_OPERAND (*expr_p, 0));
 
-  /* And remove the outermost conversion if it's useless.  */
+  /* Support C++-style dummy objects, in which void_zero is
+     cast to a pointer type.  We treat these as null pointers.  */
+  if (TREE_OPERAND (*expr_p, 0) == void_node)
+    {
+      gcc_checking_assert (POINTER_TYPE_P (TREE_TYPE (*expr_p)));
+      *expr_p = build_int_cst (TREE_TYPE (*expr_p), 0);
+    }
+
+  /* Remove the outermost conversion if it's useless.  */
   if (tree_ssa_useless_type_conversion (*expr_p))
     *expr_p = TREE_OPERAND (*expr_p, 0);