PR83648
Prathamesh Kulkarni
prathamesh.kulkarni@linaro.org
Tue Jan 9 13:06:00 GMT 2018
On 3 January 2018 at 18:58, Jan Hubicka <hubicka@ucw.cz> wrote:
>
>> diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
>> index 09ca3590039..0406d5588d2 100644
>> --- a/gcc/ipa-pure-const.c
>> +++ b/gcc/ipa-pure-const.c
>> @@ -910,7 +910,8 @@ malloc_candidate_p (function *fun, bool ipa)
>> #define DUMP_AND_RETURN(reason) \
>> { \
>> if (dump_file && (dump_flags & TDF_DETAILS)) \
>> - fprintf (dump_file, "%s", (reason)); \
>> + fprintf (dump_file, "\n%s is not a malloc candidate, reason: %s\n", \
>> + (node->name()), (reason)); \
>> return false; \
>> }
>>
>> @@ -961,11 +962,14 @@ malloc_candidate_p (function *fun, bool ipa)
>> for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
>> {
>> tree arg = gimple_phi_arg_def (phi, i);
>> + if (arg == null_pointer_node)
>> + continue;
>
> I think you want operand_equal_p here and also check for flag_delete_null_pointer_checks
> because otherwise 0 can be legal memory block addrss.
>> +
>> if (TREE_CODE (arg) != SSA_NAME)
>> - DUMP_AND_RETURN("phi arg is not SSA_NAME.")
>> - if (!(arg == null_pointer_node || check_retval_uses (arg, phi)))
>> - DUMP_AND_RETURN("phi arg has uses outside phi"
>> - " and comparisons against 0.")
>> + DUMP_AND_RETURN ("phi arg is not SSA_NAME.");
>> + if (!check_retval_uses (arg, phi))
>> + DUMP_AND_RETURN ("phi arg has uses outside phi"
>> + " and comparisons against 0.")
>
> So the case of phi(0,0) is not really correctness issue just missed optimization?
> I would still add code to handle it (it is easy).
Yes, I suppose it won't affect correctness, since it would be marked
as TOP and after propagation
it would be lowered to BOTTOM, but I added the check anyway.
>
> Do you also handle a case where parameter of phi is another phi?
Indeed, it doesn't handle multiple-phi's as in the following
test-case, good catch!
void *g (int cond1, int cond2, int cond3)
{
void *ret;
void *a;
void *b;
if (cond1)
a = __builtin_malloc (10);
else
a = __builtin_malloc (20);
if (cond2)
b = __builtin_malloc (30);
else
b = __builtin_malloc (40);
if (cond3)
ret = a;
else
ret = b;
return ret;
}
local-pure-const dump shows the function not being marked as malloc
while it should be.
I will address this issue in a follow up patch.
This patch check for flag_delete_null_pointer_checks and wheteher all
phi args are 0 in the attached patch.
Validation in progress.
Does it look OK ?
As Jakub pointed out for the case:
void *f()
{
return __builtin_malloc (0);
}
The malloc propagation would set f() to malloc.
However AFAIU, malloc(0) returns NULL (?) and the function shouldn't
be marked as malloc ?
Thanks,
Prathamesh
>
> Honza
>>
>> gimple *arg_def = SSA_NAME_DEF_STMT (arg);
>> gcall *call_stmt = dyn_cast<gcall *> (arg_def);
>> diff --git a/gcc/testsuite/gcc.dg/ipa/pr83648.c b/gcc/testsuite/gcc.dg/ipa/pr83648.c
>> new file mode 100644
>> index 00000000000..03b45de671b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/ipa/pr83648.c
>> @@ -0,0 +1,9 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-tree-local-pure-const-details" } */
>> +
>> +void *g(unsigned n)
>> +{
>> + return n ? __builtin_malloc (n) : 0;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump "Function found to be malloc: g" "local-pure-const1" } } */
>
-------------- next part --------------
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index 09ca3590039..dd15a499f6b 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -910,11 +910,13 @@ malloc_candidate_p (function *fun, bool ipa)
#define DUMP_AND_RETURN(reason) \
{ \
if (dump_file && (dump_flags & TDF_DETAILS)) \
- fprintf (dump_file, "%s", (reason)); \
+ fprintf (dump_file, "\n%s is not a malloc candidate, reason: %s\n", \
+ (node->name()), (reason)); \
return false; \
}
- if (EDGE_COUNT (exit_block->preds) == 0)
+ if (EDGE_COUNT (exit_block->preds) == 0
+ || !flag_delete_null_pointer_checks)
return false;
FOR_EACH_EDGE (e, ei, exit_block->preds)
@@ -958,34 +960,45 @@ malloc_candidate_p (function *fun, bool ipa)
}
else if (gphi *phi = dyn_cast<gphi *> (def))
- for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
- {
- tree arg = gimple_phi_arg_def (phi, i);
- if (TREE_CODE (arg) != SSA_NAME)
- DUMP_AND_RETURN("phi arg is not SSA_NAME.")
- if (!(arg == null_pointer_node || check_retval_uses (arg, phi)))
- DUMP_AND_RETURN("phi arg has uses outside phi"
- " and comparisons against 0.")
-
- gimple *arg_def = SSA_NAME_DEF_STMT (arg);
- gcall *call_stmt = dyn_cast<gcall *> (arg_def);
- if (!call_stmt)
- return false;
- tree callee_decl = gimple_call_fndecl (call_stmt);
- if (!callee_decl)
- return false;
- if (!ipa && !DECL_IS_MALLOC (callee_decl))
- DUMP_AND_RETURN("callee_decl does not have malloc attribute for"
- " non-ipa mode.")
-
- cgraph_edge *cs = node->get_edge (call_stmt);
- if (cs)
- {
- ipa_call_summary *es = ipa_call_summaries->get (cs);
- gcc_assert (es);
- es->is_return_callee_uncaptured = true;
- }
- }
+ {
+ bool nonzero_arg = false;
+
+ for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
+ {
+ tree arg = gimple_phi_arg_def (phi, i);
+ if (integer_zerop (arg))
+ continue;
+
+ if (TREE_CODE (arg) != SSA_NAME)
+ DUMP_AND_RETURN ("phi arg is not SSA_NAME.");
+ if (!check_retval_uses (arg, phi))
+ DUMP_AND_RETURN ("phi arg has uses outside phi"
+ " and comparisons against 0.")
+
+ nonzero_arg = true;
+ gimple *arg_def = SSA_NAME_DEF_STMT (arg);
+ gcall *call_stmt = dyn_cast<gcall *> (arg_def);
+ if (!call_stmt)
+ return false;
+ tree callee_decl = gimple_call_fndecl (call_stmt);
+ if (!callee_decl)
+ return false;
+ if (!ipa && !DECL_IS_MALLOC (callee_decl))
+ DUMP_AND_RETURN("callee_decl does not have malloc attribute for"
+ " non-ipa mode.")
+
+ cgraph_edge *cs = node->get_edge (call_stmt);
+ if (cs)
+ {
+ ipa_call_summary *es = ipa_call_summaries->get (cs);
+ gcc_assert (es);
+ es->is_return_callee_uncaptured = true;
+ }
+ }
+
+ if (!nonzero_arg)
+ DUMP_AND_RETURN ("all aregs to phi are 0.");
+ }
else
DUMP_AND_RETURN("def_stmt of return value is not a call or phi-stmt.")
diff --git a/gcc/testsuite/gcc.dg/ipa/pr83648.c b/gcc/testsuite/gcc.dg/ipa/pr83648.c
new file mode 100644
index 00000000000..03b45de671b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr83648.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-local-pure-const-details" } */
+
+void *g(unsigned n)
+{
+ return n ? __builtin_malloc (n) : 0;
+}
+
+/* { dg-final { scan-tree-dump "Function found to be malloc: g" "local-pure-const1" } } */
More information about the Gcc-patches
mailing list