On Fri, Feb 2, 2024 at 4:22 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Thu, Feb 01, 2024 at 03:02:47PM -0800, H.J. Lu wrote:
> > @@ -2763,6 +2789,8 @@ construct_container (machine_mode mode, machine_mode
> > orig_mode,
> > {
> > case X86_64_INTEGER_CLASS:
> > case X86_64_INTEGERSI_CLASS:
> > + if (!in_return)
> > + set_int_parameter_registers_bit (intreg[0]);
> > return gen_rtx_REG (mode, intreg[0]);
> > case X86_64_SSE_CLASS:
> > case X86_64_SSEHF_CLASS:
> > @@ -2821,6 +2849,11 @@ construct_container (machine_mode mode, machine_mode
> > orig_mode,
> > if (mode == BLKmode)
> > {
> > /* Use TImode for BLKmode values in 2 integer registers. */
> > + if (!in_return)
> > + {
> > + set_int_parameter_registers_bit (intreg[0]);
> > + set_int_parameter_registers_bit (intreg[1]);
> > + }
> > exp[0] = gen_rtx_EXPR_LIST (VOIDmode,
> > gen_rtx_REG (TImode, intreg[0]),
> > GEN_INT (0));
>
> Isn't the above (computed from just whether a function has such an argument)
> already available from cum->nregs or similar plus the sequence of argument
> registers? Or df which certainly also needs to know what registers contain
> arguments?
Fixed in v4 to use df_get_live_out instead.
> Though, the above means a function argument register which isn't used in a
> function will be impossible to use for mcount.
> We certainly can use it (although var-tracking will not know it got
> clobbered).
> So, wouldn't it be better to ask at the start of prologue generation df what
> registers are live at the start of the function (i.e. at the point of the
> NOTE_INSN_PROLOG_END because rest of prologue is emitted before that) and
> remember a suitable register for the profiling there?
> > @@ -22749,6 +22789,38 @@ current_fentry_section (const char **name)
> > return true;
> > }
> >
> > +/* Return an caller-saved register, which isn't used for parameter
> > + passing, at entry for profile. */
> > +
> > +static int
> > +x86_64_select_profile_regnum (bool r11_ok ATTRIBUTE_UNUSED)
> > +{
> > + /* Use %r10 if it isn't used by DRAP. */
> > + bool r10_ok = !crtl->drap_reg || REGNO (crtl->drap_reg) != R10_REG;
> > + if (r10_ok)
> > + return R10_REG;
> > +
> > + int i;
> > + for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> > + if (GENERAL_REGNO_P (i)
> > + && (r10_ok || i != R10_REG)
>
> r10_ok is false here, so just
> if (flag_fentry != 0 || !crtl->drap_reg || REGNO (crtl->drap_reg) !=
> R10_REG)
> return R10_REG;
> at the start and don't declare r10_ok.
>
> > +#ifdef NO_PROFILE_COUNTERS
> > + && (r11_ok || i != R11_REG)
> > +#else
> > + && i != R11_REG
> > +#endif
> > + && (!REX2_INT_REGNO_P (i) || TARGET_APX_EGPR)
> > + && !fixed_regs[i]
> > + && call_used_regs[i]
> > + && !test_int_parameter_registers_bit (i))
> > + return i;
> > +
> > + sorry ("no register available for profiling %<-mcmodel=large%s%>",
> > + ix86_cmodel == CM_LARGE_PIC ? " -fPIC" : "");
> > +
> > + return INVALID_REGNUM;
> > +}
> > +
> > /* Output assembler code to FILE to increment profiler label # LABELNO
> > for profiling a function entry. */
> > void
> > @@ -22783,42 +22855,60 @@ x86_function_profiler (FILE *file, int labelno
> > ATTRIBUTE_UNUSED)
> > fprintf (file, "\tleaq\t%sP%d(%%rip), %%r11\n", LPREFIX, labelno);
> > #endif
> >
> > + int scratch;
> > + const char *reg_prefix;
> > + const char *reg;
> > +
> > if (!TARGET_PECOFF)
> > {
> > switch (ix86_cmodel)
> > {
> > case CM_LARGE:
> > - /* NB: R10 is caller-saved. Although it can be used as a
> > - static chain register, it is preserved when calling
> > - mcount for nested functions. */
> > + scratch = x86_64_select_profile_regnum (true);
> > + reg = hi_reg_name[scratch];
> > + reg_prefix = LEGACY_INT_REGNO_P (scratch) ? "r" : "";
> > if (ASSEMBLER_DIALECT == ASM_INTEL)
> > - fprintf (file, "1:\tmovabs\tr10, OFFSET FLAT:%s\n"
> > - "\tcall\tr10\n", mcount_name);
> > + fprintf (file,
> > + "1:\tmovabs\t%s%s, OFFSET FLAT:%s\n"
> > + "\tcall\t%s%s\n",
> > + reg_prefix, reg, mcount_name, reg_prefix, reg);
> > else
> > - fprintf (file, "1:\tmovabsq\t$%s, %%r10\n\tcall\t*%%r10\n",
> > - mcount_name);
> > + fprintf (file,
> > + "1:\tmovabsq\t$%s, %%%s%s\n\tcall\t*%%%s%s\n",
> > + mcount_name, reg_prefix, reg, reg_prefix, reg);
> > break;
> > case CM_LARGE_PIC:
> > #ifdef NO_PROFILE_COUNTERS
> > + scratch = x86_64_select_profile_regnum (false);
> > + reg = hi_reg_name[scratch];
> > + reg_prefix = LEGACY_INT_REGNO_P (scratch) ? "r" : "";
> > if (ASSEMBLER_DIALECT == ASM_INTEL)
> > {
> > fprintf (file, "1:movabs\tr11, "
> > "OFFSET FLAT:_GLOBAL_OFFSET_TABLE_-1b\n");
> > - fprintf (file, "\tlea\tr10, 1b[rip]\n");
> > - fprintf (file, "\tadd\tr10, r11\n");
> > + fprintf (file, "\tlea\t%s%s, 1b[rip]\n",
> > + reg_prefix, reg);
> > + fprintf (file, "\tadd\t%s%s, r11\n",
> > + reg_prefix, reg);
> > fprintf (file, "\tmovabs\tr11, OFFSET FLAT:%s@PLTOFF\n",
> > mcount_name);
> > - fprintf (file, "\tadd\tr10, r11\n");
> > - fprintf (file, "\tcall\tr10\n");
> > + fprintf (file, "\tadd\t%s%s, r11\n",
> > + reg_prefix, reg);
> > + fprintf (file, "\tcall\t%s%s\n",
> > + reg_prefix, reg);
> > break;
> > }
> > fprintf (file,
> > "1:\tmovabsq\t$_GLOBAL_OFFSET_TABLE_-1b, %%r11\n");
> > - fprintf (file, "\tleaq\t1b(%%rip), %%r10\n");
> > - fprintf (file, "\taddq\t%%r11, %%r10\n");
> > + fprintf (file, "\tleaq\t1b(%%rip), %%%s%s\n",
> > + reg_prefix, reg);
> > + fprintf (file, "\taddq\t%%r11, %%%s%s\n",
> > + reg_prefix, reg);
> > fprintf (file, "\tmovabsq\t$%s@PLTOFF, %%r11\n", mcount_name);
> > - fprintf (file, "\taddq\t%%r11, %%r10\n");
> > - fprintf (file, "\tcall\t*%%r10\n");
> > + fprintf (file, "\taddq\t%%r11, %%%s%s\n",
> > + reg_prefix, reg);
> > + fprintf (file, "\tcall\t*%%%s%s\n",
> > + reg_prefix, reg);
> > #else
> > sorry ("profiling %<-mcmodel=large%> with PIC is not
> > supported");
> > #endif
> > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> > index 35ce8b00d36..7967bc5196c 100644
> > --- a/gcc/config/i386/i386.h
> > +++ b/gcc/config/i386/i386.h
> > @@ -2847,6 +2847,11 @@ struct GTY(()) machine_function {
> > /* True if red zone is used. */
> > BOOL_BITFIELD red_zone_used : 1;
> >
> > + /* Bit mask for integer registers used for parameter passing.
> > + The lower 8 bits are for legacy registers and the upper
> > + 8 bits are for r8-r15. */
> > + unsigned int int_parameter_registers : 16;
> > +
> > /* The largest alignment, in bytes, of stack slot actually used. */
> > unsigned int max_used_stack_alignment;
> >
> > diff --git a/gcc/testsuite/gcc.target/i386/pr113689-1.c
> > b/gcc/testsuite/gcc.target/i386/pr113689-1.c
> > new file mode 100644
> > index 00000000000..c32445e0fc4
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr113689-1.c
> > @@ -0,0 +1,49 @@
> > +/* { dg-do run { target { lp64 && fpic } } } */
> > +/* { dg-options "-O2 -fno-pic -fprofile -mcmodel=large" } */
> > +
> > +#include <stdarg.h>
> > +
> > +__attribute__((noipa,noclone,noinline))
> > +void
> > +bar (int a1, int a2, int a3, int a4, int a5, int a6,
> > + char *x, char *y, int *z)
> > +{
> > + if (a1 != 1)
> > + __builtin_abort ();
> > + if (a2 != 2)
> > + __builtin_abort ();
> > + if (a3 != 3)
> > + __builtin_abort ();
> > + if (a4 != 4)
> > + __builtin_abort ();
> > + if (a5 != 5)
> > + __builtin_abort ();
> > + if (a6 != 6)
> > + __builtin_abort ();
> > + x[0] = 42;
> > + y[0] = 42;
> > + if (z[0] != 16)
> > + __builtin_abort ();
> > +}
> > +
> > +__attribute__((noipa,noclone,noinline))
> > +void
> > +foo (int c, int d, int e, int f, int g, int h, int z, ...)
> > +{
> > + typedef char B[32];
> > + B b __attribute__((aligned (32)));
> > + va_list ap;
> > + va_start (ap, z);
> > + int x = va_arg (ap, int);
> > + if (x != 38)
> > + __builtin_abort ();
> > + bar (c, d, e, f, g, h, &b[0], __builtin_alloca (z), &z);
> > + va_end (ap);
> > +}
> > +
> > +int
> > +main ()
> > +{
> > + foo (1, 2, 3, 4, 5, 6, 16, 38);
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/i386/pr113689-2.c
> > b/gcc/testsuite/gcc.target/i386/pr113689-2.c
> > new file mode 100644
> > index 00000000000..fec5b171a39
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr113689-2.c
> > @@ -0,0 +1,41 @@
> > +/* { dg-do run { target { lp64 && fpic } } } */
> > +/* { dg-options "-O2 -fpic -fprofile -mcmodel=large" } */
> > +
> > +__attribute__((noipa,noclone,noinline))
> > +void
> > +bar (int a1, int a2, int a3, int a4, int a5, int a6,
> > + char *x, char *y, int *z)
> > +{
> > + if (a1 != 1)
> > + __builtin_abort ();
> > + if (a2 != 2)
> > + __builtin_abort ();
> > + if (a3 != 3)
> > + __builtin_abort ();
> > + if (a4 != 4)
> > + __builtin_abort ();
> > + if (a5 != 5)
> > + __builtin_abort ();
> > + if (a6 != 6)
> > + __builtin_abort ();
> > + x[0] = 42;
> > + y[0] = 42;
> > + if (z[0] != 16)
> > + __builtin_abort ();
> > +}
> > +
> > +__attribute__((noipa,noclone,noinline))
> > +void
> > +foo (int c, int d, int e, int f, int g, int h, int z)
> > +{
> > + typedef char B[32];
> > + B b __attribute__((aligned (32)));
> > + bar (c, d, e, f, g, h, &b[0], __builtin_alloca (z), &z);
> > +}
> > +
> > +int
> > +main ()
> > +{
> > + foo (1, 2, 3, 4, 5, 6, 16);
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/i386/pr113689-3.c
> > b/gcc/testsuite/gcc.target/i386/pr113689-3.c
> > new file mode 100644
> > index 00000000000..10099fc8d96
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr113689-3.c
> > @@ -0,0 +1,24 @@
> > +/* { dg-do run { target { lp64 && avx_runtime } } } */
>
> I think this test isn't needed.
> If you really want it, it should have bitint effective
> target and guard with __BITINT_MAXWIDTH__ >= 511 to be
> consistent with other bitint tests.
Removed in v4.
> > +/* { dg-options "-O2 -fprofile -mcmodel=large -mavx" } */
> > +
> > +__attribute__((noipa,noclone,noinline))
> > +_BitInt(511)
> > +foo(_BitInt(7) a, _BitInt(511) b)
> > +{
> > + ({
> > + volatile _BitInt(511) d = (-b / a);
> > + if (d != -1)
> > + __builtin_abort();
> > + d;
> > + });
> > + return b;
> > +}
> > +
> > +int
> > +main(void)
> > +{
> > + _BitInt(511) x = foo(5, 5);
> > + if (x != 5)
> > + __builtin_abort();
> > + return 0;
> > +}
> > --
> > 2.43.0
>
> Jakub
>
--
H.J.
