| Message ID | mptmue773d9.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add expr_callee_abi | expand |
On 10/11/19 8:39 AM, Richard Sandiford wrote: > This turned out to be useful for the SVE PCS support, and is a natural > tree-level analogue of insn_callee_abi. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > Richard > > > 2019-10-11 Richard Sandiford <richard.sandiford@arm.com> > > gcc/ > * function-abi.h (expr_callee_abi): Declare. > * function-abi.cc (expr_callee_abi): New function. OK. Yes I realize you're not using it on the trunk yet :-) jeff
On Fri, Oct 11, 2019 at 4:39 PM Richard Sandiford <richard.sandiford@arm.com> wrote: > > This turned out to be useful for the SVE PCS support, and is a natural > tree-level analogue of insn_callee_abi. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > Richard > > > 2019-10-11 Richard Sandiford <richard.sandiford@arm.com> > > gcc/ > * function-abi.h (expr_callee_abi): Declare. > * function-abi.cc (expr_callee_abi): New function. > > Index: gcc/function-abi.h > =================================================================== > --- gcc/function-abi.h 2019-09-30 17:39:33.514597856 +0100 > +++ gcc/function-abi.h 2019-10-11 15:38:54.141605718 +0100 > @@ -315,5 +315,6 @@ call_clobbered_in_region_p (unsigned int > extern const predefined_function_abi &fntype_abi (const_tree); > extern function_abi fndecl_abi (const_tree); > extern function_abi insn_callee_abi (const rtx_insn *); > +extern function_abi expr_callee_abi (const_tree); > > #endif > Index: gcc/function-abi.cc > =================================================================== > --- gcc/function-abi.cc 2019-09-30 17:39:33.514597856 +0100 > +++ gcc/function-abi.cc 2019-10-11 15:38:54.141605718 +0100 > @@ -229,3 +229,32 @@ insn_callee_abi (const rtx_insn *insn) > > return default_function_abi; > } > + > +/* Return the ABI of the function called by CALL_EXPR EXP. Return the > + default ABI for erroneous calls. */ > + > +function_abi > +expr_callee_abi (const_tree exp) > +{ > + gcc_assert (TREE_CODE (exp) == CALL_EXPR); > + > + if (tree fndecl = get_callee_fndecl (exp)) > + return fndecl_abi (fndecl); Please not. The ABI in effect on the call is that of the type of CALL_EXPR_FN, what GIMPLE optimizers propagated as fndecl here doesn't matter. > + > + tree callee = CALL_EXPR_FN (exp); > + if (callee == error_mark_node) > + return default_function_abi; > + > + tree type = TREE_TYPE (callee); > + if (type == error_mark_node) > + return default_function_abi; > + > + if (POINTER_TYPE_P (type)) > + { > + type = TREE_TYPE (type); > + if (type == error_mark_node) > + return default_function_abi; > + } so when it's not a POINTER_TYPE (it always shold be!) then you're handing arbitrary types to fntype_abi. > + return fntype_abi (type); > +}
Richard Biener <richard.guenther@gmail.com> writes: > On Fri, Oct 11, 2019 at 4:39 PM Richard Sandiford > <richard.sandiford@arm.com> wrote: >> >> This turned out to be useful for the SVE PCS support, and is a natural >> tree-level analogue of insn_callee_abi. >> >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? >> >> Richard >> >> >> 2019-10-11 Richard Sandiford <richard.sandiford@arm.com> >> >> gcc/ >> * function-abi.h (expr_callee_abi): Declare. >> * function-abi.cc (expr_callee_abi): New function. >> >> Index: gcc/function-abi.h >> =================================================================== >> --- gcc/function-abi.h 2019-09-30 17:39:33.514597856 +0100 >> +++ gcc/function-abi.h 2019-10-11 15:38:54.141605718 +0100 >> @@ -315,5 +315,6 @@ call_clobbered_in_region_p (unsigned int >> extern const predefined_function_abi &fntype_abi (const_tree); >> extern function_abi fndecl_abi (const_tree); >> extern function_abi insn_callee_abi (const rtx_insn *); >> +extern function_abi expr_callee_abi (const_tree); >> >> #endif >> Index: gcc/function-abi.cc >> =================================================================== >> --- gcc/function-abi.cc 2019-09-30 17:39:33.514597856 +0100 >> +++ gcc/function-abi.cc 2019-10-11 15:38:54.141605718 +0100 >> @@ -229,3 +229,32 @@ insn_callee_abi (const rtx_insn *insn) >> >> return default_function_abi; >> } >> + >> +/* Return the ABI of the function called by CALL_EXPR EXP. Return the >> + default ABI for erroneous calls. */ >> + >> +function_abi >> +expr_callee_abi (const_tree exp) >> +{ >> + gcc_assert (TREE_CODE (exp) == CALL_EXPR); >> + >> + if (tree fndecl = get_callee_fndecl (exp)) >> + return fndecl_abi (fndecl); > > Please not. The ABI in effect on the call is that of > the type of CALL_EXPR_FN, what GIMPLE optimizers > propagated as fndecl here doesn't matter. expr_callee_abi is returning the ABI of the callee function, ignoring any additional effects involved in actually calling it. It's supposed to take advantage of local IPA information where possible, e.g. by ignoring call-clobbered registers that the target doesn't actually clobber. So if get_callee_fndecl returns either the correct FUNCTION_DECL or null, using it gives better information than just using the type. Failing to propagate the decl (or not having a decl to propagate) just means that we don't take advantage of the IPA information. >> + >> + tree callee = CALL_EXPR_FN (exp); >> + if (callee == error_mark_node) >> + return default_function_abi; >> + >> + tree type = TREE_TYPE (callee); >> + if (type == error_mark_node) >> + return default_function_abi; >> + >> + if (POINTER_TYPE_P (type)) >> + { >> + type = TREE_TYPE (type); >> + if (type == error_mark_node) >> + return default_function_abi; >> + } > > so when it's not a POINTER_TYPE (it always shold be!) > then you're handing arbitrary types to fntype_abi. fntype_abi asserts that TYPE is an appropriate type. There's no safe value we can return if it isn't. (Well, apart from the error_mark_node cases above, where the assumption is that compilation is going to fail anyway.) I can change it to assert for POINTER_TYPE_P if that's guaranteed everywhere. Thanks, Richard > >> + return fntype_abi (type); >> +}
On October 14, 2019 2:53:36 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote: >Richard Biener <richard.guenther@gmail.com> writes: >> On Fri, Oct 11, 2019 at 4:39 PM Richard Sandiford >> <richard.sandiford@arm.com> wrote: >>> >>> This turned out to be useful for the SVE PCS support, and is a >natural >>> tree-level analogue of insn_callee_abi. >>> >>> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? >>> >>> Richard >>> >>> >>> 2019-10-11 Richard Sandiford <richard.sandiford@arm.com> >>> >>> gcc/ >>> * function-abi.h (expr_callee_abi): Declare. >>> * function-abi.cc (expr_callee_abi): New function. >>> >>> Index: gcc/function-abi.h >>> =================================================================== >>> --- gcc/function-abi.h 2019-09-30 17:39:33.514597856 +0100 >>> +++ gcc/function-abi.h 2019-10-11 15:38:54.141605718 +0100 >>> @@ -315,5 +315,6 @@ call_clobbered_in_region_p (unsigned int >>> extern const predefined_function_abi &fntype_abi (const_tree); >>> extern function_abi fndecl_abi (const_tree); >>> extern function_abi insn_callee_abi (const rtx_insn *); >>> +extern function_abi expr_callee_abi (const_tree); >>> >>> #endif >>> Index: gcc/function-abi.cc >>> =================================================================== >>> --- gcc/function-abi.cc 2019-09-30 17:39:33.514597856 +0100 >>> +++ gcc/function-abi.cc 2019-10-11 15:38:54.141605718 +0100 >>> @@ -229,3 +229,32 @@ insn_callee_abi (const rtx_insn *insn) >>> >>> return default_function_abi; >>> } >>> + >>> +/* Return the ABI of the function called by CALL_EXPR EXP. Return >the >>> + default ABI for erroneous calls. */ >>> + >>> +function_abi >>> +expr_callee_abi (const_tree exp) >>> +{ >>> + gcc_assert (TREE_CODE (exp) == CALL_EXPR); >>> + >>> + if (tree fndecl = get_callee_fndecl (exp)) >>> + return fndecl_abi (fndecl); >> >> Please not. The ABI in effect on the call is that of >> the type of CALL_EXPR_FN, what GIMPLE optimizers >> propagated as fndecl here doesn't matter. > >expr_callee_abi is returning the ABI of the callee function, ignoring >any additional effects involved in actually calling it. It's supposed >to take advantage of local IPA information where possible, e.g. by >ignoring call-clobbered registers that the target doesn't actually >clobber. > >So if get_callee_fndecl returns either the correct FUNCTION_DECL or >null, >using it gives better information than just using the type. Failing to >propagate the decl (or not having a decl to propagate) just means that >we don't take advantage of the IPA information. So we don't ever want to use this for stdcall vs. Msabi Or so where the indirect call function type might be correctly attributed while a function decl is not? >>> + >>> + tree callee = CALL_EXPR_FN (exp); >>> + if (callee == error_mark_node) >>> + return default_function_abi; >>> + >>> + tree type = TREE_TYPE (callee); >>> + if (type == error_mark_node) >>> + return default_function_abi; >>> + >>> + if (POINTER_TYPE_P (type)) >>> + { >>> + type = TREE_TYPE (type); >>> + if (type == error_mark_node) >>> + return default_function_abi; >>> + } >> >> so when it's not a POINTER_TYPE (it always shold be!) >> then you're handing arbitrary types to fntype_abi. > >fntype_abi asserts that TYPE is an appropriate type. There's no safe >value we can return if it isn't. (Well, apart from the error_mark_node >cases above, where the assumption is that compilation is going to fail >anyway.) > >I can change it to assert for POINTER_TYPE_P if that's guaranteed >everywhere. That's probably better. Thanks, Richard. >Thanks, >Richard > >> >>> + return fntype_abi (type); >>> +}
Richard Biener <richard.guenther@gmail.com> writes: > On October 14, 2019 2:53:36 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote: >>Richard Biener <richard.guenther@gmail.com> writes: >>> On Fri, Oct 11, 2019 at 4:39 PM Richard Sandiford >>> <richard.sandiford@arm.com> wrote: >>>> >>>> This turned out to be useful for the SVE PCS support, and is a >>natural >>>> tree-level analogue of insn_callee_abi. >>>> >>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? >>>> >>>> Richard >>>> >>>> >>>> 2019-10-11 Richard Sandiford <richard.sandiford@arm.com> >>>> >>>> gcc/ >>>> * function-abi.h (expr_callee_abi): Declare. >>>> * function-abi.cc (expr_callee_abi): New function. >>>> >>>> Index: gcc/function-abi.h >>>> =================================================================== >>>> --- gcc/function-abi.h 2019-09-30 17:39:33.514597856 +0100 >>>> +++ gcc/function-abi.h 2019-10-11 15:38:54.141605718 +0100 >>>> @@ -315,5 +315,6 @@ call_clobbered_in_region_p (unsigned int >>>> extern const predefined_function_abi &fntype_abi (const_tree); >>>> extern function_abi fndecl_abi (const_tree); >>>> extern function_abi insn_callee_abi (const rtx_insn *); >>>> +extern function_abi expr_callee_abi (const_tree); >>>> >>>> #endif >>>> Index: gcc/function-abi.cc >>>> =================================================================== >>>> --- gcc/function-abi.cc 2019-09-30 17:39:33.514597856 +0100 >>>> +++ gcc/function-abi.cc 2019-10-11 15:38:54.141605718 +0100 >>>> @@ -229,3 +229,32 @@ insn_callee_abi (const rtx_insn *insn) >>>> >>>> return default_function_abi; >>>> } >>>> + >>>> +/* Return the ABI of the function called by CALL_EXPR EXP. Return >>the >>>> + default ABI for erroneous calls. */ >>>> + >>>> +function_abi >>>> +expr_callee_abi (const_tree exp) >>>> +{ >>>> + gcc_assert (TREE_CODE (exp) == CALL_EXPR); >>>> + >>>> + if (tree fndecl = get_callee_fndecl (exp)) >>>> + return fndecl_abi (fndecl); >>> >>> Please not. The ABI in effect on the call is that of >>> the type of CALL_EXPR_FN, what GIMPLE optimizers >>> propagated as fndecl here doesn't matter. >> >>expr_callee_abi is returning the ABI of the callee function, ignoring >>any additional effects involved in actually calling it. It's supposed >>to take advantage of local IPA information where possible, e.g. by >>ignoring call-clobbered registers that the target doesn't actually >>clobber. >> >>So if get_callee_fndecl returns either the correct FUNCTION_DECL or >>null, >>using it gives better information than just using the type. Failing to >>propagate the decl (or not having a decl to propagate) just means that >>we don't take advantage of the IPA information. > > So we don't ever want to use this for stdcall vs. Msabi Or so where the indirect call function type might be correctly attributed while a function decl is not? Right. This wouldn't be the right query for that case, since the function is only telling us about the target. >>>> + >>>> + tree callee = CALL_EXPR_FN (exp); >>>> + if (callee == error_mark_node) >>>> + return default_function_abi; >>>> + >>>> + tree type = TREE_TYPE (callee); >>>> + if (type == error_mark_node) >>>> + return default_function_abi; >>>> + >>>> + if (POINTER_TYPE_P (type)) >>>> + { >>>> + type = TREE_TYPE (type); >>>> + if (type == error_mark_node) >>>> + return default_function_abi; >>>> + } >>> >>> so when it's not a POINTER_TYPE (it always shold be!) >>> then you're handing arbitrary types to fntype_abi. >> >>fntype_abi asserts that TYPE is an appropriate type. There's no safe >>value we can return if it isn't. (Well, apart from the error_mark_node >>cases above, where the assumption is that compilation is going to fail >>anyway.) >> >>I can change it to assert for POINTER_TYPE_P if that's guaranteed >>everywhere. > > That's probably better. OK, here's what I applied after retesting with the ACLE stuff that uses it. Thanks, Richard 2019-10-16 Richard Sandiford <richard.sandiford@arm.com> gcc/ * function-abi.cc (expr_callee_abi): Assert for POINTER_TYPE_P. Index: gcc/function-abi.cc =================================================================== --- gcc/function-abi.cc 2019-10-14 09:05:57.142808555 +0100 +++ gcc/function-abi.cc 2019-10-16 11:58:35.362779932 +0100 @@ -249,12 +249,6 @@ expr_callee_abi (const_tree exp) if (type == error_mark_node) return default_function_abi; - if (POINTER_TYPE_P (type)) - { - type = TREE_TYPE (type); - if (type == error_mark_node) - return default_function_abi; - } - - return fntype_abi (type); + gcc_assert (POINTER_TYPE_P (type)); + return fntype_abi (TREE_TYPE (type)); }
Index: gcc/function-abi.h =================================================================== --- gcc/function-abi.h 2019-09-30 17:39:33.514597856 +0100 +++ gcc/function-abi.h 2019-10-11 15:38:54.141605718 +0100 @@ -315,5 +315,6 @@ call_clobbered_in_region_p (unsigned int extern const predefined_function_abi &fntype_abi (const_tree); extern function_abi fndecl_abi (const_tree); extern function_abi insn_callee_abi (const rtx_insn *); +extern function_abi expr_callee_abi (const_tree); #endif Index: gcc/function-abi.cc =================================================================== --- gcc/function-abi.cc 2019-09-30 17:39:33.514597856 +0100 +++ gcc/function-abi.cc 2019-10-11 15:38:54.141605718 +0100 @@ -229,3 +229,32 @@ insn_callee_abi (const rtx_insn *insn) return default_function_abi; } + +/* Return the ABI of the function called by CALL_EXPR EXP. Return the + default ABI for erroneous calls. */ + +function_abi +expr_callee_abi (const_tree exp) +{ + gcc_assert (TREE_CODE (exp) == CALL_EXPR); + + if (tree fndecl = get_callee_fndecl (exp)) + return fndecl_abi (fndecl); + + tree callee = CALL_EXPR_FN (exp); + if (callee == error_mark_node) + return default_function_abi; + + tree type = TREE_TYPE (callee); + if (type == error_mark_node) + return default_function_abi; + + if (POINTER_TYPE_P (type)) + { + type = TREE_TYPE (type); + if (type == error_mark_node) + return default_function_abi; + } + + return fntype_abi (type); +}