From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id C99703857C51 for ; Fri, 2 Feb 2024 12:22:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C99703857C51 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C99703857C51 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706876564; cv=none; b=LZKxaxi4cIdEqizrvPDEAsoPciAl9fpTcbI11TLIwIHaedDqQ1ktvpZmc+YGxISAUOxRgFhS/s743BgZv71gd+W9mzgCo1dX1uu5dAvb7Ny6X+ZvHMddMpmlsEAlA/bMriYPoS590FFxvTegA6vQYF3Plr4PE4BE4BJURjRluFQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706876564; c=relaxed/simple; bh=guYFH9KNHrVxX2zXzKKLqkImY2Aa058JqYTtw/bOv+Q=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=v3fWfxsnNFkbU1Izq0Hwo7jdDogKRqbHB+d26BZa4oeM2SqYMeguBFEe5Vb66VNISN4yvtDSq2kEpP4L8J8bzzNOSTXaCWBVZsxNwNoo+e16h46tTZRP/u1jVfzabqaLrT9rYVFyrgZHkuIjkYbIZroEDOmcqaRxxI54WBNDm1g= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706876562; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=B37RfXi62vvvz3sd56cu8JcQl5Hehzv3S6Iufp3Ry20=; b=Z+g7icr06stDOw8WOzen/fKfiCOFItYlqB38mLXqvQIsDnjDfzjEyfOenoMEJuy++qLHcZ 3puQ4zNAUu8SyBT+dVW+pEo2Lo/VQ4QZjqM99RPs5/MJXsvXeV4QH3LIN8pOe9f5z++qAy 55aWoxbL/SRXm4uWrEHX1geqVYzcvC4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-487-XkxmkOuTM9CbZcJTZqbISQ-1; Fri, 02 Feb 2024 07:22:39 -0500 X-MC-Unique: XkxmkOuTM9CbZcJTZqbISQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1166183538A; Fri, 2 Feb 2024 12:22:39 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C629D2166B31; Fri, 2 Feb 2024 12:22:38 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 412CMWGW2722099 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 2 Feb 2024 13:22:32 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 412CMVHV2722098; Fri, 2 Feb 2024 13:22:31 +0100 Date: Fri, 2 Feb 2024 13:22:31 +0100 From: Jakub Jelinek To: "H.J. Lu" Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH v2] x86-64: Find a scratch register for large model profiling Message-ID: Reply-To: Jakub Jelinek References: <20240201230247.214664-1-hjl.tools@gmail.com> MIME-Version: 1.0 In-Reply-To: <20240201230247.214664-1-hjl.tools@gmail.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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 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? 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 > + > +__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. > +/* { 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