From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x533.google.com (mail-ed1-x533.google.com [IPv6:2a00:1450:4864:20::533]) by sourceware.org (Postfix) with ESMTPS id CFE323858CDB for ; Mon, 5 Feb 2024 10:56:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CFE323858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CFE323858CDB Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::533 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707130582; cv=none; b=np0cGmchcY187shunPUdsuOmGHsog0MI3GYxPvbg/klqyZzr0+op6PAcC70l4uHdooeWrMfI+b9nuoUXdXU0C9yP/XXxYQrhmtfK+LtW9VI6ZDkAZrE3ACFAWuh/B1uJd0K1UTU3uRkgxPK41ZUkcLgLgZeb+4r2F5NNA0qK5rk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707130582; c=relaxed/simple; bh=ZUJjZmdM7cE4dEUz5Dq0QnuFe5n7aZzd8Z7b7YJxQhI=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=B2uyA/DZi7aGyjGvutM/8DlUjbexFm/AIWRa4X3ZwyjJanGPyxhYbdX8W7JDJMVWbc+LVERSwTbpWM9puzhz1apWhUxFMNvtjCF+hjHBsW4jPcHQvROdq9uFNXnTsBgkaZcCvFGqMt+tivnju1VZefb4TEVCwCKpxNHeswzMcGE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ed1-x533.google.com with SMTP id 4fb4d7f45d1cf-56061ad3d3dso1374950a12.1 for ; Mon, 05 Feb 2024 02:56:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1707130578; x=1707735378; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=FNQk0rD5DwxiFd0AJODXTndQxoFUjnFSlBlvsJEMFYo=; b=Ihss/GylR08/25UFZs6gHC/GPB43SzG6Kd0TZrP07186IxMk9E2L5On1rlpI8CXLDe vQHaPbb0jfMVAsDH8/UxxAiLGcWgzUNSFsZ7e9u0e6xQoibLqseRAL3PtM5SjJO0k6BY Du/BQZSiRlDIxJ4ywWLb5LbrlYSrFhf373vmQ521YuhlR9v39cxtK1qbmRiBW3LYiECH pWwWmgwTvtsmmYYeeSvYz8oLAejSFKV4acQrddMfiNp2Olo4FLdv+dZ3OFap5uzJLubD tpMc2WpkcSt6FbUcyKcIxkI/evtndNVf1wSp2UsGBxM4x5s3u4CUmF6tcAQ/R1YLhJRE Q+6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707130578; x=1707735378; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=FNQk0rD5DwxiFd0AJODXTndQxoFUjnFSlBlvsJEMFYo=; b=WaicC/pokHJkJOYCdcDuOose9LfpEafZTp/RE+9/CAQWyWkfEDD+NYSJo785X7AKDt Sf7PxHIkezbRQR2z60nqd4CRg82TioKmEwo7jIQ7x0UobcY4DtSXdPtY744NNyQGaNHc Ut93piaDAHQcwhrqlJ3YF/z4tkgB9dAqqJ+D+47EtALtNR8xctFzvai1ZZgoyrSUd30Z 1NKFLmE9/eMSE/+yHAeiwKWxj3IppCdVynfzJi3L7WcbfovY9kDFBfRjxug6fA4m69lU w8dA/g0sNWAjZckcDaMicQvYwai+FZrZ5Z9XZ8DgxE+9EKvmctNRWyT9sTnoFTY1nrlx ut9A== X-Gm-Message-State: AOJu0Yy8wkRtOXKvZBV5dKll2fgtk9B7Pxc50OcWDTEZ9i8NGvMkewIC FIT3DyKIrQfGpBfDMs1HsblER+s7br5Zd2Zn6ThQwB6XuGEq/+C7GlP12FwxRPGipPPDWU4uBUS O+oQZSfqs6AMi/jTaZAuQetQPwcU= X-Google-Smtp-Source: AGHT+IHZqExfaCdd9+NVULsHQ9LiJD0UoOR3VMFcOYRgzfb5lEU9FcUHhBNLILReuoR+hkNJ1sOvGwdmCpDKAcBwqW8= X-Received: by 2002:a05:6402:b30:b0:55f:c8e4:4c09 with SMTP id bo16-20020a0564020b3000b0055fc8e44c09mr5653668edb.14.1707130578135; Mon, 05 Feb 2024 02:56:18 -0800 (PST) MIME-Version: 1.0 References: <20240202224708.343462-1-hjl.tools@gmail.com> In-Reply-To: <20240202224708.343462-1-hjl.tools@gmail.com> From: Uros Bizjak Date: Mon, 5 Feb 2024 11:56:06 +0100 Message-ID: Subject: Re: [PATCH v5] x86-64: Find a scratch register for large model profiling To: "H.J. Lu" Cc: gcc-patches@gcc.gnu.org, rep.dot.nop@gmail.com, jakub@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, Feb 2, 2024 at 11:47=E2=80=AFPM H.J. Lu wrote= : > > Changes in v5: > > 1. Add pr113689-3.c. > 2. Use %r10 if ix86_profile_before_prologue () return true. > 3. Try a callee-saved register which has been saved on stack in the > prologue. > > Changes in v4: > > 1. Remove pr113689-3.c. > 2. Use df_get_live_out. > > Changes in v3: > > 1. Remove r10_ok. > > Changes in v2: > > 1. Add int_parameter_registers to machine_function to track integer > registers used for parameter passing. > 2. Update x86_64_select_profile_regnum to try %r10 first and use an > caller-saved register, which isn't used for parameter passing. > > --- > 2 scratch registers, %r10 and %r11, are available at function entry for > large model profiling. But %r10 may be used by stack realignment and we > can't use %r10 in this case. Add x86_64_select_profile_regnum to find > a caller-saved register which isn't live or a callee-saved register > which has been saved on stack in the prologue at entry for large model > profiling and sorry if we can't find one. > > gcc/ > > PR target/113689 > * config/i386/i386.cc (set_saved_int_registers_bit): New. > (test_saved_int_registers_bit): Likewise. > (ix86_emit_save_regs): Call set_saved_int_registers_bit on > saved register. > (ix86_emit_save_regs_using_mov): Likewise. > (x86_64_select_profile_regnum): New. > (x86_function_profiler): Call x86_64_select_profile_regnum to > get a scratch register for large model profiling. > * config/i386/i386.h (machine_function): Add > saved_int_registers. > > gcc/testsuite/ > > PR target/113689 > * gcc.target/i386/pr113689-1.c: New file. > * gcc.target/i386/pr113689-2.c: Likewise. > * gcc.target/i386/pr113689-3.c: Likewise. > --- > gcc/config/i386/i386.cc | 119 ++++++++++++++++++--- > gcc/config/i386/i386.h | 5 + > gcc/testsuite/gcc.target/i386/pr113689-1.c | 49 +++++++++ > gcc/testsuite/gcc.target/i386/pr113689-2.c | 41 +++++++ > gcc/testsuite/gcc.target/i386/pr113689-3.c | 48 +++++++++ > 5 files changed, 247 insertions(+), 15 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr113689-1.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr113689-2.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr113689-3.c > > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc > index b3e7c74846e..1c7aaa4535e 100644 > --- a/gcc/config/i386/i386.cc > +++ b/gcc/config/i386/i386.cc > @@ -7387,6 +7387,32 @@ choose_baseaddr (HOST_WIDE_INT cfa_offset, unsigne= d int *align, > return plus_constant (Pmode, base_reg, base_offset); > } > > +/* Set the integer register REGNO bit in saved_int_registers. */ > + > +static void > +set_saved_int_registers_bit (int regno) > +{ > + if (LEGACY_INT_REGNO_P (regno)) > + cfun->machine->saved_int_registers |=3D 1 << regno; > + else > + cfun->machine->saved_int_registers > + |=3D 1 << (regno - FIRST_REX_INT_REG + 8); > +} > + > +/* Return true if the integer register REGNO bit in saved_int_registers > + is set. */ > + > +static bool > +test_saved_int_registers_bit (int regno) > +{ > + if (LEGACY_INT_REGNO_P (regno)) > + return (cfun->machine->saved_int_registers > + & (1 << regno)) !=3D 0; > + else > + return (cfun->machine->saved_int_registers > + & (1 << (regno - FIRST_REX_INT_REG + 8))) !=3D 0; > +} > + > /* Emit code to save registers in the prologue. */ > > static void > @@ -7403,6 +7429,7 @@ ix86_emit_save_regs (void) > insn =3D emit_insn (gen_push (gen_rtx_REG (word_mode, regno), > TARGET_APX_PPX)); > RTX_FRAME_RELATED_P (insn) =3D 1; > + set_saved_int_registers_bit (regno); > } > } > else > @@ -7415,6 +7442,7 @@ ix86_emit_save_regs (void) > for (regno =3D FIRST_PSEUDO_REGISTER - 1; regno >=3D 0; regno--) > if (GENERAL_REGNO_P (regno) && ix86_save_reg (regno, true, true)) > { > + set_saved_int_registers_bit (regno); > if (aligned) > { > regno_list[loaded_regnum++] =3D regno; > @@ -7567,6 +7595,7 @@ ix86_emit_save_regs_using_mov (HOST_WIDE_INT cfa_of= fset) > { > ix86_emit_save_reg_using_mov (word_mode, regno, cfa_offset); > cfa_offset -=3D UNITS_PER_WORD; > + set_saved_int_registers_bit (regno); > } > } Do we really need the above handling? I think that we can use ix86_save_reg directly in x86_64_select_profile_regnum below. > @@ -22749,6 +22778,48 @@ current_fentry_section (const char **name) > return true; > } > > +/* Return a caller-saved register which isn't live or a callee-saved > + register which has been saved on stack in the prologue at entry for > + profile. */ > + > +static int > +x86_64_select_profile_regnum (bool r11_ok ATTRIBUTE_UNUSED) > +{ > + /* Use %r10 if the profiler is emitted before the prologue or it isn't > + used by DRAP. */ > + if (ix86_profile_before_prologue () > + || !crtl->drap_reg > + || REGNO (crtl->drap_reg) !=3D R10_REG) > + return R10_REG; > + > + /* The profiler is emitted after the prologue. If there is a > + caller-saved register which isn't live or a callee-saved > + register saved on stack in the prologue, use it. */ > + > + bitmap reg_live =3D df_get_live_out (ENTRY_BLOCK_PTR_FOR_FN (cfun)); > + > + int i; > + for (i =3D 0; i < FIRST_PSEUDO_REGISTER; i++) > + if (GENERAL_REGNO_P (i) > + && i !=3D R10_REG > +#ifdef NO_PROFILE_COUNTERS > + && (r11_ok || i !=3D R11_REG) > +#else > + && i !=3D R11_REG > +#endif > + && (!REX2_INT_REGNO_P (i) || TARGET_APX_EGPR) > + && !fixed_regs[i] > + && (test_saved_int_registers_bit (i) > + || (call_used_regs[i] > + && !REGNO_REG_SET_P (reg_live, i)))) > + return i; We are scanning the whole hard reg space, so we can use ix86_save_reg here instead of introducing test_saved_int_registers_bit array. Also, you can use accessible_reg_set to determine which general register set is available. > + > + sorry ("no register available for profiling %<-mcmodel=3Dlarge%s%>", > + ix86_cmodel =3D=3D 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 +22854,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 =3D x86_64_select_profile_regnum (true); > + reg =3D hi_reg_name[scratch]; > + reg_prefix =3D LEGACY_INT_REGNO_P (scratch) ? "r" : ""; Oh... please rather construct a complete reg name here and avoid using reg_prefix below. > if (ASSEMBLER_DIALECT =3D=3D 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 =3D x86_64_select_profile_regnum (false); > + reg =3D hi_reg_name[scratch]; > + reg_prefix =3D LEGACY_INT_REGNO_P (scratch) ? "r" : ""; Also here, please construct a complete reg name here. Uros. > if (ASSEMBLER_DIALECT =3D=3D 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_nam= e); > - 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=3Dlarge%> with PIC is not supp= orted"); > #endif > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h > index 35ce8b00d36..33821a04074 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 saved on stack in prologue. The > + lower 8 bits are for legacy registers and the upper 8 bits are > + for r8-r15. */ > + unsigned int saved_int_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/g= cc.target/i386/pr113689-1.c > new file mode 100644 > index 00000000000..8285c0a07b7 > --- /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=3Dlarge" } */ > + > +#include > + > +__attribute__((noipa)) > +void > +bar (int a1, int a2, int a3, int a4, int a5, int a6, > + char *x, char *y, int *z) > +{ > + if (a1 !=3D 1) > + __builtin_abort (); > + if (a2 !=3D 2) > + __builtin_abort (); > + if (a3 !=3D 3) > + __builtin_abort (); > + if (a4 !=3D 4) > + __builtin_abort (); > + if (a5 !=3D 5) > + __builtin_abort (); > + if (a6 !=3D 6) > + __builtin_abort (); > + x[0] =3D 42; > + y[0] =3D 42; > + if (z[0] !=3D 16) > + __builtin_abort (); > +} > + > +__attribute__((noipa)) > +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); > + double x =3D va_arg (ap, double); > + if (x > 40.0) > + __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.0); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.target/i386/pr113689-2.c b/gcc/testsuite/g= cc.target/i386/pr113689-2.c > new file mode 100644 > index 00000000000..2e5579ac546 > --- /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=3Dlarge" } */ > + > +__attribute__((noipa)) > +void > +bar (int a1, int a2, int a3, int a4, int a5, int a6, > + char *x, char *y, int *z) > +{ > + if (a1 !=3D 1) > + __builtin_abort (); > + if (a2 !=3D 2) > + __builtin_abort (); > + if (a3 !=3D 3) > + __builtin_abort (); > + if (a4 !=3D 4) > + __builtin_abort (); > + if (a5 !=3D 5) > + __builtin_abort (); > + if (a6 !=3D 6) > + __builtin_abort (); > + x[0] =3D 42; > + y[0] =3D 42; > + if (z[0] !=3D 16) > + __builtin_abort (); > +} > + > +__attribute__((noipa)) > +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/g= cc.target/i386/pr113689-3.c > new file mode 100644 > index 00000000000..dab75190635 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr113689-3.c > @@ -0,0 +1,48 @@ > +/* { dg-do run { target { lp64 && fpic } } } */ > +/* { dg-options "-O2 -fpic -fprofile -mcmodel=3Dlarge" } */ > + > +#include > + > +__attribute__((noipa)) > +void > +bar (char *x, char *y, int *z) > +{ > + x[0] =3D 42; > + y[0] =3D 42; > + if (z[0] !=3D 16) > + __builtin_abort (); > +} > + > +__attribute__((noipa)) > +void > +foo (int a1, int a2, int a3, int a4, int a5, int a6, int z, ...) > +{ > + typedef char B[32]; > + B b __attribute__((aligned (32))); > + va_list ap; > + va_start (ap, z); > + double x =3D va_arg (ap, double); > + if (x > 40.0) > + __builtin_abort (); > + if (a1 !=3D 1) > + __builtin_abort (); > + if (a2 !=3D 2) > + __builtin_abort (); > + if (a3 !=3D 3) > + __builtin_abort (); > + if (a4 !=3D 4) > + __builtin_abort (); > + if (a5 !=3D 5) > + __builtin_abort (); > + if (a6 !=3D 6) > + __builtin_abort (); > + bar (&b[0], __builtin_alloca (z), &z); > + va_end (ap); > +} > + > +int > +main () > +{ > + foo (1, 2, 3, 4, 5, 6, 16, 38.0); > + return 0; > +} > -- > 2.43.0 >