| Message ID | AM5PR0802MB261001F326E111092BC1780983E20@AM5PR0802MB2610.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 12 May 2017, Wilco Dijkstra wrote: > This is a followup from: https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02916.html > > Add an assert to leaf_function_p to ensure it is not called from a > prolog or epilog sequence (which would incorrectly return true in a > non-leaf function). As I understand, we need to ensure that get_insns call retrieves the topmost sequence corresponding to the function body, not any current subsequence that could have been started via start_sequence. Therefore the 'prolog or epilog' part is a bit misleading, we could be in a subsequence for other reasons, and we need to reject those as well. So, ... > diff --git a/gcc/final.c b/gcc/final.c > index 820162b2d28d734901375017cf0c7a3095e8903e..c9aa610d2696738342f61b9c944a7a2f18e7497c 100644 > --- a/gcc/final.c > +++ b/gcc/final.c > @@ -4309,6 +4309,9 @@ leaf_function_p (void) > { > rtx_insn *insn; > > + /* Check we are not in a prologue or epilogue sequence. */ > + gcc_assert (!in_sequence_p ()); > + ... can the comment please be reworded to match the code, if it's necessary to have a comment here at all? E.g. "Ensure we walk the entire function body after the following get_insns call". Thanks. Alexander
On 05/12/2017 07:30 AM, Alexander Monakov wrote: > On Fri, 12 May 2017, Wilco Dijkstra wrote: > >> This is a followup from: https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02916.html >> >> Add an assert to leaf_function_p to ensure it is not called from a >> prolog or epilog sequence (which would incorrectly return true in a >> non-leaf function). > > As I understand, we need to ensure that get_insns call retrieves the topmost > sequence corresponding to the function body, not any current subsequence that > could have been started via start_sequence. Therefore the 'prolog or epilog' > part is a bit misleading, we could be in a subsequence for other reasons, and > we need to reject those as well. So, ... Right. It's not really a prologue/epilogue issue, but anytime we call leaf_function_p and we're not at the topmost sequence there's probably a bug. jeff
diff --git a/gcc/final.c b/gcc/final.c index 820162b2d28d734901375017cf0c7a3095e8903e..c9aa610d2696738342f61b9c944a7a2f18e7497c 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -4309,6 +4309,9 @@ leaf_function_p (void) { rtx_insn *insn; + /* Check we are not in a prologue or epilogue sequence. */ + gcc_assert (!in_sequence_p ()); + /* Some back-ends (e.g. s390) want leaf functions to stay leaf functions even if they call mcount. */ if (crtl->profile && !targetm.keep_leaf_when_profiled ())