Re: ipa-inline & what TARGET_CAN_INLINE_P can assume

[Richard Sandiford <richard.sandiford@arm.com>]

Andrew Pinski <pinskia@gmail.com> writes:
> On Mon, Sep 25, 2023 at 10:16 AM Richard Sandiford via Gcc
> <gcc@gcc.gnu.org> wrote:
>>
>> Hi,
>>
>> I have a couple of questions about what TARGET_CAN_INLINE_P is
>> alllowed to assume when called from ipa-inline.  (Callers from the
>> front-end don't matter for the moment.)
>>
>> I'm working on an extension where a function F1 without attribute A
>> can't be inlined into a function F2 with attribute A.  That part is
>> easy and standard.
>>
>> But it's expected that many functions won't have attribute A,
>> even if they could.  So we'd like to detect automatically whether
>> F1's implementation is compatible with attribute A.  This is something
>> we can do by scanning the gimple code.
>>
>> However, even if we detect that F1's code is compatible with attribute A,
>> we don't want to add attribute A to F1 itself because (a) it would change
>> F1's ABI and (b) it would restrict the optimisation of any non-inlined
>> copy of F1.  So this is a test for inlining only.
>>
>> TARGET_CAN_INLINE_P (F2, F1) can check whether F1's current code
>> is compatible with attribute A.  But:
>>
>> (a) Is it safe to assume (going forward) that F1 won't change before
>>     it is inlined into F2?  Specifically, is it safe to assume that
>>     nothing will be inlined into F1 between the call to TARGET_CAN_INLINE_P
>>     and the inlining of F1 into F2?
>>
>> (b) For compile-time reasons, I'd like to cache the result in
>>     machine_function.  The cache would be a three-state:
>>
>>     - not tested
>>     - compatible with A
>>     - incompatible with A
>>
>>     The cache would be reset to "not tested" whenever TARGET_CAN_INLINE_P
>>     is called with F1 as the *caller* rather than the callee.  The idea
>>     is to handle cases where something is inlined into F1 after F1 has
>>     been inlined into F2.  (This would include calls from the main
>>     inlining pass, after the early pass has finished.)
>>
>>     Is resetting the cache in this way sufficient?  Or should we have a
>>     new interface for this?
>>
>> Sorry for the long question :)  I have something that seems to work,
>> but I'm not sure whether it's misusing the interface.
>
>
> The rs6000 backend has a similar issue and defined the following
> target hooks which seems exactly what you need in this case
> TARGET_NEED_IPA_FN_TARGET_INFO
> TARGET_UPDATE_IPA_FN_TARGET_INFO
>
> And then use that information in can_inline_p target hook to mask off
> the ISA bits:
>       unsigned int info = ipa_fn_summaries->get (callee_node)->target_info;
>       if ((info & RS6000_FN_TARGET_INFO_HTM) == 0)
>         {
>           callee_isa &= ~OPTION_MASK_HTM;
>           explicit_isa &= ~OPTION_MASK_HTM;
>         }

Thanks!  Like you say, it looks like a perfect fit.

The optimisation of having TARGET_UPDATE_IPA_FN_TARGET_INFO return false
to stop further analysis probably won't trigger for this use case.
I need to track two conditions and the second one is very rare.
But that's still going to be much better than potentially scanning
the same (inlined) stmts multiple times.

Richard