Re: [PATCH] Fix missed DSE opportunity with operator delete.

[Richard Biener]

On Tue, Apr 19, 2016 at 10:48 PM, Mikhail Maltsev <malts...@gmail.com> wrote:
> On 04/18/2016 12:14 PM, Richard Biener wrote:
>>
>> Enlarging tree_function_decl is bad.
> Probably using 3 bits for malloc_flag, operator_new_flag and free_flag is
> redundant. I packed the state into 2 bits.
>>
>> Passes should get at the info via flags_from_decl_or_type () and a new
>> ECF_FREE.
> Fixed.

Thanks - much better.

@@ -2117,6 +2127,13 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref)
          /* Fallthru to general call handling.  */;
       }

+  if (callee != NULL_TREE
+      && (flags_from_decl_or_type (callee) & ECF_FREE) != 0)
+    {

as you have a stmt here please use gimple_call_flags (call) & ECF_FREE.

@@ -2402,6 +2409,16 @@ stmt_kills_ref_p (gimple *stmt, ao_ref *ref)

          default:;
          }
+
+      if (callee != NULL_TREE
+         && (flags_from_decl_or_type (callee) & ECF_FREE) != 0)
+       {

Likewise.

@@ -1728,6 +1729,15 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *ref)
          /* Fallthru to general call handling.  */;
       }

+  /* free-like functions may not reference their first argument.  */
+  if (callee != NULL_TREE && (flags & ECF_FREE) != 0)
+    {
+      tree ptr = gimple_call_arg (call, 0);
+      tree base = ao_ref_base (ref);
+      if (base && TREE_CODE (base) == MEM_REF && TREE_OPERAND (base, 0) == ptr)
+       return false;
+    }
+

So this is less aggressive than what we do for BUILT_IN_FREE which simply
returns false as "not reading from (any) memory".  I suspect we might want
to amend the documentation of the "free" attribute to that effect, or find
a better wording ...

Otherwise using DECL_SET_MALLOC sometimes and sometimes
DECL_ALLOC_FN_KIND () = ALLOC_FN_MALLOC looks somewhat
inconsistent.  I'd prefer removing DECL_SET_MALLOC.

You have

+static tree
+handle_free_attribute (tree *node, tree name, tree /*args*/, int /*flags*/,
+                      bool *no_add_attrs)
+{
+  tree decl = *node;
+  if (TREE_CODE (decl) == FUNCTION_DECL
+      && type_num_arguments (TREE_TYPE (decl)) != 0
+      && POINTER_TYPE_P (TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (decl)))))
+    DECL_ALLOC_FN_KIND (decl) = ALLOC_FN_FREE;
+  else
+    {
+      warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+                 "%qE attribute ignored", name);
+      *no_add_attrs = true;
+    }

so one can happily apply the attribute to

 void foo (void *, void *);

but then

@@ -2117,6 +2127,13 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref)
          /* Fallthru to general call handling.  */;
       }

+  if (callee != NULL_TREE
+      && (flags_from_decl_or_type (callee) & ECF_FREE) != 0)
+    {
+      tree ptr = gimple_call_arg (call, 0);
+      return ptr_deref_may_alias_ref_p_1 (ptr, ref);
+    }

will ignore the 2nd argument.  I think it's better to ignore the attribute
if type_num_arguments () != 1.

Richard.

> --
> Regards,
>     Mikhail Maltsev