Re: ipa-inline & what TARGET_CAN_INLINE_P can assume

2023-09-25 Thread Richard Sandiford via Gcc
Andrew Pinski  writes:
> On Mon, Sep 25, 2023 at 10:16 AM Richard Sandiford via Gcc
>  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


Re: ipa-inline & what TARGET_CAN_INLINE_P can assume

2023-09-25 Thread Andrew Pinski via Gcc
On Mon, Sep 25, 2023 at 10:16 AM Richard Sandiford via Gcc
 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,
Andrew Pinski


>
> Thanks,
> Richard


ipa-inline & what TARGET_CAN_INLINE_P can assume

2023-09-25 Thread Richard Sandiford via Gcc
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.

Thanks,
Richard