[Bug target/104327] New: [12 Regression] Inlining error on s390x since r12-1039

[Bug target/104327] New: [12 Regression] Inlining error on s390x since r12-1039

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

            Bug ID: 104327
           Summary: [12 Regression] Inlining error on s390x since r12-1039
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
  Target Milestone: ---

Since r12-1039-gebd5e86c0f41dc1d692f9b2b68a510b1f6835a3e
we reject following testcase on s390x-linux (reduced from php):
void foo (int *);

static inline __attribute__((always_inline)) void
bar (int *x)
{
  foo (x);
}

__attribute__((cold, optimize("Os"))) void
baz (int *x)
{
  bar (x);
}
(with -O0 or -O2 etc.):
rh2048565.c: In function ‘baz’:
rh2048565.c:4:1: error: inlining failed in call to ‘always_inline’ ‘bar’:
target specific option mismatch
    4 | bar (int *x)
      | ^~~
rh2048565.c:12:3: note: called from here
   12 |   bar (x);
      |   ^~~~~~~

target attribute isn't used at all though.  Seems the mismatch is because the
r12-1039 code sets DECL_FUNCTION_SPECIFIC_TARGET on the baz caller to non-NULL,
while bar has it NULL.

[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
           Priority|P3                          |P1
                 CC|                            |krebbel at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org

[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

Martin Liška <marxin at gcc dot gnu.org> 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 <marxin at gcc dot gnu.org> ---
Lemme take a look.

[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |s390x

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
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

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #5 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
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

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

Martin Liška <marxin at gcc dot gnu.org> 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 <marxin at gcc dot gnu.org> ---
(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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #8 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
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

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andreas Krebbel <krebbel@gcc.gnu.org>:

https://gcc.gnu.org/g:db95441cf5399aabc46ca83df19f7290c3e23cb1

commit r12-7081-gdb95441cf5399aabc46ca83df19f7290c3e23cb1
Author: Andreas Krebbel <krebbel@linux.ibm.com>
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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug target/104327] [12 Regression] Inlining error on s390x since r12-1039

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Georg-Johann Lay <gjl@gcc.gnu.org>:

https://gcc.gnu.org/g:66cc0cb0f44f17049f61af6755043999c4fa5a24

commit r14-1245-g66cc0cb0f44f17049f61af6755043999c4fa5a24
Author: Georg-Johann Lay <avr@gjlay.de>
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

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Georg-Johann Lay
<gjl@gcc.gnu.org>:

https://gcc.gnu.org/g:6506590e70e57ed8d7fb68ab9443e31c31208fb0

commit r13-7378-g6506590e70e57ed8d7fb68ab9443e31c31208fb0
Author: Georg-Johann Lay <avr@gjlay.de>
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

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104327

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Georg-Johann Lay
<gjl@gcc.gnu.org>:

https://gcc.gnu.org/g:0537c71aa7ef88c4ffe754cf7af81e346273b079

commit r12-9655-g0537c71aa7ef88c4ffe754cf7af81e346273b079
Author: Georg-Johann Lay <avr@gjlay.de>
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.