[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 --- Comment #13 from CVS Commits --- The releases/gcc-12 branch has been updated by Georg-Johann Lay : https://gcc.gnu.org/g:0537c71aa7ef88c4ffe754cf7af81e346273b079 commit r12-9655-g0537c71aa7ef88c4ffe754cf7af81e346273b079 Author: Georg-Johann Lay Date: Tue May 23 14:54:12 2023 +0200 target/104327: Allow more inlining between different optimization levels. avr-common.cc introduces the following options that are set depending on optimization level: -mgas-isr-prologues, -mmain-is-OS-task and -fsplit-wide-types-early. The inliner thinks that different options disallow cross-optimization inlining, so provide can_inline_p. gcc/ PR target/104327 * config/avr/avr.cc (avr_can_inline_p): New static function. (TARGET_CAN_INLINE_P): Define to that function.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 --- Comment #12 from CVS Commits --- The releases/gcc-13 branch has been updated by Georg-Johann Lay : https://gcc.gnu.org/g:6506590e70e57ed8d7fb68ab9443e31c31208fb0 commit r13-7378-g6506590e70e57ed8d7fb68ab9443e31c31208fb0 Author: Georg-Johann Lay Date: Tue May 23 14:54:12 2023 +0200 target/104327: Allow more inlining between different optimization levels. avr-common.cc introduces the following options that are set depending on optimization level: -mgas-isr-prologues, -mmain-is-OS-task and -fsplit-wide-types-early. The inliner thinks that different options disallow cross-optimization inlining, so provide can_inline_p. gcc/ PR target/104327 * config/avr/avr.cc (avr_can_inline_p): New static function. (TARGET_CAN_INLINE_P): Define to that function.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 --- Comment #11 from CVS Commits --- The master branch has been updated by Georg-Johann Lay : https://gcc.gnu.org/g:66cc0cb0f44f17049f61af6755043999c4fa5a24 commit r14-1245-g66cc0cb0f44f17049f61af6755043999c4fa5a24 Author: Georg-Johann Lay Date: Tue May 23 14:54:12 2023 +0200 target/104327: Allow more inlining between different optimization levels. avr-common.cc introduces the following options that are set depending on optimization level: -mgas-isr-prologues, -mmain-is-OS-task and -fsplit-wide-types-early. The inliner thinks that different options disallow cross-optimization inlining, so provide can_inline_p. gcc/ PR target/104327 * config/avr/avr.cc (avr_can_inline_p): New static function. (TARGET_CAN_INLINE_P): Define to that function.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 Jakub Jelinek changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #10 from Jakub Jelinek --- Fixed.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 --- Comment #9 from CVS Commits --- The master branch has been updated by Andreas Krebbel : https://gcc.gnu.org/g:db95441cf5399aabc46ca83df19f7290c3e23cb1 commit r12-7081-gdb95441cf5399aabc46ca83df19f7290c3e23cb1 Author: Andreas Krebbel Date: Sun Feb 6 09:07:41 2022 +0100 Check always_inline flag in s390_can_inline_p [PR104327] MASK_MVCLE is set for -Os but not for other optimization levels. In general it should not make much sense to inline across calls where the flag is different but we have to allow it for always_inline. The patch also rearranges the hook implementation a bit based on the recommendations from Jakub und Martin in the PR. Bootstrapped and regression tested on s390x with various arch flags. Will commit after giving a few days for comments. gcc/ChangeLog: PR target/104327 * config/s390/s390.cc (s390_can_inline_p): Accept a few more flags if always_inline is set. Don't inline when tune differs without always_inline. gcc/testsuite/ChangeLog: PR target/104327 * gcc.c-torture/compile/pr104327.c: New test.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 --- Comment #8 from Andreas Krebbel --- I will work on a patch. Thanks for the hint! I agree for HTM. VX is an ABI switch since it changes the calling conventions for vector types.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 --- Comment #7 from Jakub Jelinek --- The testcase should be in a generic directory, so that we catch it on other targets too. Note, e.g. i386/ can_inline_p hook is quite different from this one. E.g. for the ISA options it allows the caller to have a superset of the ISAs: /* Callee's isa options should be a subset of the caller's, i.e. a SSE4 function can inline a SSE2 function but a SSE2 function can't inline a SSE4 function. */ if (((caller_opts->x_ix86_isa_flags & callee_opts->x_ix86_isa_flags) != callee_opts->x_ix86_isa_flags) || ((caller_opts->x_ix86_isa_flags2 & callee_opts->x_ix86_isa_flags2) != callee_opts->x_ix86_isa_flags2)) ret = false; which is heavily relied on (e.g. most of the intrinsics have some ISA and we allow inlining if the caller supports those), takes into account whether it always_inline callee, etc. Now, s390 does something like that e.g. with the arch. target_flags seem to be a mixed bag of features: #define MASK_64BIT (1U << 0) #define MASK_BACKCHAIN (1U << 1) #define MASK_DEBUG_ARG (1U << 2) #define MASK_ZARCH (1U << 3) #define MASK_HARD_DFP (1U << 4) #define MASK_SOFT_FLOAT (1U << 5) #define MASK_OPT_HTM (1U << 6) #define MASK_LONG_DOUBLE_128 (1U << 7) #define MASK_MVCLE (1U << 8) #define MASK_PACKED_STACK (1U << 9) #define MASK_SMALL_EXEC (1U << 10) #define MASK_OPT_VX (1U << 11) #define MASK_ZVECTOR (1U << 12) I'd say e.g. OPT_VX or OPT_HTM seem to me like ISA flags that could be treated that way, MVCLE is an tuning option from what I understand (s390 doesn't check *_tune at all, maybe it could for non-always_inline?), e.g. 64BIT or LONG_DOUBLE_128 seems to be an ABI option so any changes should make code uninlinable, ...
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 Martin Liška changed: What|Removed |Added Status|ASSIGNED|NEW Assignee|marxin at gcc dot gnu.org |unassigned at gcc dot gnu.org --- Comment #6 from Martin Liška --- (In reply to Andreas Krebbel from comment #5) > Yes, that's the right fix I think. Thanks! > MVCLE is a shorter version of a loop doing MVCs but has some startup > overhead. I would factor it out a bit: diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc index 2db12d4ba4b..95351c8a002 100644 --- a/gcc/config/s390/s390.cc +++ b/gcc/config/s390/s390.cc @@ -16105,8 +16105,10 @@ s390_can_inline_p (tree caller, tree callee) struct cl_target_option *callee_opts = TREE_TARGET_OPTION (callee_tree); bool ret = true; - if ((caller_opts->x_target_flags & ~(MASK_SOFT_FLOAT | MASK_HARD_DFP)) - != (callee_opts->x_target_flags & ~(MASK_SOFT_FLOAT | MASK_HARD_DFP))) + unsigned HOST_WIDE_INT inline_safe_mask += MASK_SOFT_FLOAT | MASK_HARD_DFP | MASK_MVCLE; + if ((caller_opts->x_target_flags & ~inline_safe_mask) + != (callee_opts->x_target_flags & ~inline_safe_mask)) ret = false; /* Don't inline functions to be compiled for a more recent arch into a @Andreas: Can you please add a test case for it, test it and finish the patch? Thanks.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 --- Comment #5 from Andreas Krebbel --- Yes, that's the right fix I think. Thanks! MVCLE is a shorter version of a loop doing MVCs but has some startup overhead.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 --- Comment #4 from Jakub Jelinek --- The mvcle instructions actually aren't guarded by TARGET_MVCLE, they are TARGET_64BIT || !TARGET_ZARCH or !TARGET_64BIT && TARGET_ZARCH which means available everywhere, so TARGET_MVCLE seems just like an optimization hint, defaulted for -Os, but which can be set separately. So, I'd say we should use if ((caller_opts->x_target_flags & ~(MASK_SOFT_FLOAT | MASK_HARD_DFP | MASK_MVCLE)) != (callee_opts->x_target_flags & ~(MASK_SOFT_FLOAT | MASK_HARD_DFP | MASK_MVCLE))) ret = false; or so.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327
--- Comment #3 from Martin Liška ---
So target specific option mismatch is caused by:
static const struct default_options s390_option_optimization_table[] =
{
...
/* Use MVCLE instructions to decrease code size if requested. */
{ OPT_LEVELS_SIZE, OPT_mmvcle, NULL, 1 },
...
which can be very likely handled in:
static bool
s390_can_inline_p (tree caller, tree callee)
{
...
if ((caller_opts->x_target_flags & ~(MASK_SOFT_FLOAT | MASK_HARD_DFP))
!= (callee_opts->x_target_flags & ~(MASK_SOFT_FLOAT | MASK_HARD_DFP)))
ret = false;
@Andreas: What do you think?
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 Richard Biener changed: What|Removed |Added Target||s390x --- Comment #2 from Richard Biener --- Guess this will hit all targets that have the bogus code special-casing NULL DECL_FUNCTION_SPECIFIC_TARGET in their target hook instead of using the global options node for that case.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 Martin Liška changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Ever confirmed|0 |1 Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org Last reconfirmed||2022-02-01 --- Comment #1 from Martin Liška --- Lemme take a look.
[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327 Jakub Jelinek changed: What|Removed |Added Target Milestone|--- |12.0 Priority|P3 |P1 CC||krebbel at gcc dot gnu.org, ||marxin at gcc dot gnu.org
