On Wed, Mar 26, 2025 at 09:01:43AM +0100, Jakub Jelinek wrote:
> The following testcase FAILs because the always_inline function can't
> be inlined.
> The rs6000 backend has similarly to other targets a hook which rejects
> inlining which would bring in new ISAs which aren't there in the caller.
> And this hook rejects this because of OPTION_MASK_SAVE_TOC_INDIRECT
> differences.
> This flag is set if explicitly requested or by default depending on
> whether the current function looks hot (or at least not cold):
> if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0
> && flag_shrink_wrap_separate
> && optimize_function_for_speed_p (cfun))
> rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;
> The target nodes that are being compared here are actually the default
> target node (which was created when cfun was NULL) vs. one that was
> created for the always_inline function when it wasn't NULL, so one
> doesn't have it, the other does.
> In any case, this flag feels like a tuning decision rather than hard
> ISA requirement and I see no problems why we couldn't inline
> even explicit -msave-toc-indirect function into -mno-save-toc-indirect
> or vice versa.
> We already ignore OPTION_MASK_P{8,10}_FUSION which are also more
> like tuning flags.
It is okay for trunk. Thank you! (And thanks to Surya, for making it
the focus of my attention).,
> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
>
> 2025-03-26 Jakub Jelinek <[email protected]>
>
> PR target/119327
> * config/rs6000/rs6000.cc (rs6000_can_inline_p): Ignore also
> OPTION_MASK_SAVE_TOC_INDIRECT differences.
>
> * g++.dg/opt/pr119327.C: New test.
>
> --- gcc/config/rs6000/rs6000.cc.jj 2025-03-18 14:56:37.990023768 +0100
> +++ gcc/config/rs6000/rs6000.cc 2025-03-25 13:21:33.174568536 +0100
> @@ -25765,10 +25765,13 @@ rs6000_can_inline_p (tree caller, tree c
> }
> }
>
> - /* Ignore -mpower8-fusion and -mpower10-fusion options for inlining
> - purposes. */
> - callee_isa &= ~(OPTION_MASK_P8_FUSION | OPTION_MASK_P10_FUSION);
> - explicit_isa &= ~(OPTION_MASK_P8_FUSION | OPTION_MASK_P10_FUSION);
> + /* Ignore -mpower8-fusion, -mpower10-fusion and -msave-toc-indirect options
> + for inlining purposes. */
> + HOST_WIDE_INT ignored_isas = (OPTION_MASK_P8_FUSION
> + | OPTION_MASK_P10_FUSION
The P8/P10 fusion parts are obviously correct. The "TOC fusion" part
is harder, but that should never heve been a user option at all, it
either is okay or it isn't.
Maybe that thing should not be user-selectable at all.
("Maybe", like I guarantee I will okay a patch doing jsut that!)
(Not "would", but "will", thanks)
So yeah, tha patch is okay. Thank you!
Segher
