| Message ID | CAAgBjMmAmZHjNKJkkzjJU_QmGH-FQ5i2qawN8PYuMj7-n82=9A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR85817 | expand |
On Fri, 18 May 2018, Prathamesh Kulkarni wrote: > Hi, > In r260250, the condition > > if (integer_zerop (retval)) > continue; > > was added before checking retval was of pointer type which caused > functions having return type apart from void *, to be marked as > malloc. The attached patch gets rid of the above check since we do not > wish to mark function returning NULL as malloc. > Also, it adds a check to return false if all args to phi are 0, > although I am not sure if this'd actually trigger in practice since > constant propagation should have folded the phi into constant 0 > already. > > Bootstrap+test in progress on x86_64-linux-gnu and aarch64-linux-gnu. > OK to commit if passes ? OK. Thanks, Richard.
On 05/18/2018 02:49 AM, Prathamesh Kulkarni wrote: > Hi, > In r260250, the condition > > if (integer_zerop (retval)) > continue; > > was added before checking retval was of pointer type which caused > functions having return type apart from void *, to be marked as > malloc. The attached patch gets rid of the above check since we do not > wish to mark function returning NULL as malloc. > Also, it adds a check to return false if all args to phi are 0, > although I am not sure if this'd actually trigger in practice since > constant propagation should have folded the phi into constant 0 > already. > > Bootstrap+test in progress on x86_64-linux-gnu and aarch64-linux-gnu. > OK to commit if passes ? FWIW, I'm currently digging into a bootstrap failure on riscv64 that is triggered by the original change to allow functions returning NULL to potentially be malloc candidates. I'll give things a spin with this patch as well. jeff
On 05/18/2018 02:49 AM, Prathamesh Kulkarni wrote: > Hi, > In r260250, the condition > > if (integer_zerop (retval)) > continue; > > was added before checking retval was of pointer type which caused > functions having return type apart from void *, to be marked as > malloc. The attached patch gets rid of the above check since we do not > wish to mark function returning NULL as malloc. > Also, it adds a check to return false if all args to phi are 0, > although I am not sure if this'd actually trigger in practice since > constant propagation should have folded the phi into constant 0 > already. > > Bootstrap+test in progress on x86_64-linux-gnu and aarch64-linux-gnu. > OK to commit if passes ? So I just checked and it appears this will resolve the riscv64 issue. The additional marking was ultimately causing the alias analysis code to narrow the aliasing scope of a local variable that had its address taken -- to the point where it was no longer considered escaping. With the variable no longer escaping, calls were no longer may-defs of the variable and Wuninitialized could perform a more precise analysis and determined the variable was used potentially uninitialized. In reality the uninitialized use is guarded in such a way that it won't be used uninitialized, but without some kind of late inlining the analysis wouldn't be able to see the use is properly guarded. With the more restrictive marking, the variable is considered escaping again and thus is may-def'd by random calls and we don't trigger the -Wuninitialized warning anymore. Thanks, Jeff
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c index 567b615fb60..528ea6695ac 100644 --- a/gcc/ipa-pure-const.c +++ b/gcc/ipa-pure-const.c @@ -940,9 +940,6 @@ malloc_candidate_p (function *fun, bool ipa) if (!retval) DUMP_AND_RETURN("No return value.") - if (integer_zerop (retval)) - continue; - if (TREE_CODE (retval) != SSA_NAME || TREE_CODE (TREE_TYPE (retval)) != POINTER_TYPE) DUMP_AND_RETURN("Return value is not SSA_NAME or not a pointer type.") @@ -972,37 +969,44 @@ 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 (integer_zerop (arg)) - continue; + { + bool all_args_zero = true; + for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i) + { + tree arg = gimple_phi_arg_def (phi, i); + if (integer_zerop (arg)) + continue; + + all_args_zero = false; + 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.") + + 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 (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.") - - 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 (all_args_zero) + DUMP_AND_RETURN ("Return value is a phi with all args equal to 0."); + } else DUMP_AND_RETURN("def_stmt of return value is not a call or phi-stmt.") diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr83648.c b/gcc/testsuite/gcc.dg/tree-ssa/pr83648.c index febfd7d9319..884faf81167 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/pr83648.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr83648.c @@ -12,4 +12,4 @@ void *h() } /* { dg-final { scan-tree-dump "Function found to be malloc: g" "local-pure-const1" } } */ -/* { dg-final { scan-tree-dump "Function found to be malloc: h" "local-pure-const1" } } */ +/* { dg-final { scan-tree-dump-not "Function found to be malloc: h" "local-pure-const1" } } */