diff mbox series

Mark necessary 2nd and later args for delete op.

Message ID ac24740b-bbfa-c19a-e416-e072198e6c54@suse.cz
State New
Headers show
Series Mark necessary 2nd and later args for delete op. | expand

Commit Message

Martin Liška July 31, 2019, 8:37 a.m. UTC 
On 7/30/19 4:00 PM, Marc Glisse wrote:
> On Tue, 30 Jul 2019, Martin Liška wrote:
> 
>> Ah, that's bad, both of them need a care:
> 
> Yes, that makes more sense to me, thanks.
> 
>>> I tried to experiment to understand, but it is complicated because including <new> disables the optimization:
>>>
>>> #include <new>
>>> void fn1() {
>>>     char*p=new char;
>>>     delete p;
>>> }
>>>
>>> This ICEs with -O -std=c++17:
>>>
>>> int a = 64;
>>> std::align_val_t b{64};
>>> void fn1() {
>>>   void *s = operator new(a,b);
>>>   operator delete(s,8+*(unsigned long*)s,b);
>>> }
>>>
>>>
>>
>> I can't see it on current master. Can you?
> 
> Yes. I just did a clean build to make sure.
> 

That's addressed in a patch I'm attaching.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

Comments

Richard Biener July 31, 2019, 10:02 a.m. UTC | #1 
On Wed, Jul 31, 2019 at 10:37 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 7/30/19 4:00 PM, Marc Glisse wrote:
> > On Tue, 30 Jul 2019, Martin Liška wrote:
> >
> >> Ah, that's bad, both of them need a care:
> >
> > Yes, that makes more sense to me, thanks.
> >
> >>> I tried to experiment to understand, but it is complicated because including <new> disables the optimization:
> >>>
> >>> #include <new>
> >>> void fn1() {
> >>>     char*p=new char;
> >>>     delete p;
> >>> }
> >>>
> >>> This ICEs with -O -std=c++17:
> >>>
> >>> int a = 64;
> >>> std::align_val_t b{64};
> >>> void fn1() {
> >>>   void *s = operator new(a,b);
> >>>   operator delete(s,8+*(unsigned long*)s,b);
> >>> }
> >>>
> >>>
> >>
> >> I can't see it on current master. Can you?
> >
> > Yes. I just did a clean build to make sure.
> >
>
> That's addressed in a patch I'm attaching.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

OK.

> Thanks,
> Martin
diff mbox series

Patch

From cd975aaccbbed3c03bbd246e0802c94693a0962d Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Tue, 30 Jul 2019 15:40:24 +0200
Subject: [PATCH] Mark necessary 2nd and later args for delete op.

gcc/ChangeLog:

2019-07-30  Martin Liska  <mliska@suse.cz>

	* tree-ssa-dce.c (propagate_necessity): Delete operator can
	have size and (or) alignment as 2nd and later arguments.
	Mark all of them as necessary.
---
 gcc/tree-ssa-dce.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index bec13cd5930..80d5f5c30f7 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -824,13 +824,16 @@  propagate_necessity (bool aggressive)
 			   || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
 		      || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)))
 		{
-		  /* Some delete operators have size as 2nd argument.  */
+		  /* Delete operators can have alignment and (or) size as next
+		     arguments.  When being a SSA_NAME, they must be marked
+		     as necessary.  */
 		  if (is_delete_operator && gimple_call_num_args (stmt) >= 2)
-		    {
-		      tree size_argument = gimple_call_arg (stmt, 1);
-		      if (TREE_CODE (size_argument) == SSA_NAME)
-			mark_operand_necessary (size_argument);
-		    }
+		    for (unsigned i = 1; i < gimple_call_num_args (stmt); i++)
+		      {
+			tree arg = gimple_call_arg (stmt, i);
+			if (TREE_CODE (arg) == SSA_NAME)
+			  mark_operand_necessary (arg);
+		      }
 
 		  continue;
 		}
-- 
2.22.0