From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by sourceware.org (Postfix) with ESMTPS id C031B385E001 for ; Fri, 7 Oct 2022 04:52:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C031B385E001 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dabbelt.com Received: by mail-pl1-x633.google.com with SMTP id d24so3579536pls.4 for ; Thu, 06 Oct 2022 21:52:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:from:to:cc:subject:date:message-id :reply-to; bh=df8stucJzGyugdL0IPw0NwBsGgahhQX3IROVBIeq3gk=; b=b5t4yeRgfmjHcLw1PjHAh4n8iftEdY5gybpEP/CzlzqvHmD2NefcydUpi+hMlE7Lgs ik0Dw7G2cnkf1pyqckngZLmSucmxpkiWPE1qso+4LPfzIrLxH1KyvcOpX35fBRq7L/h0 cq3TD1NR3L/nZwYsA8ZY5f9ESvbS/6uwxPf66q6iHe/MRX8XZLz//Sr7MkvdOXQVdHGX a4RqHxX/BWhKX9MA9BmaZhZWYzolufKhB2JR7ptZcs+tD2j3xsorcwsuz3NA7OTKJH0W 1DXtdejZw3bxfWg16/I87Qjq2GXR2CjvJ9D8Itc8Th1VdkJaroaFzc21RBoIObLtnY8U 2lfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=df8stucJzGyugdL0IPw0NwBsGgahhQX3IROVBIeq3gk=; b=kvoN1MvkaXFs0Vp+uNqmYfcyxBCDhp1EOtvTArwWvNHdD1YvOYjaneuG8m0G+T5IBG XNt9VawtHemf7fOPlWQPGYIS2ngaiPAyaYzluKnFMmJqprPtOBez5CeUnuCbefL7o77d TknVQeO0RMyESDswr1WIgef6aOcf5Ukxzs3wVldghQ0VDNvruTLP+ZmWWrxzkjHTF4YV 26FmHfQYZCk3pv2d2nPSbH89A6pP9RLb/DgHbCYKgRZ70PmoZlTgmUxiL9hLmL/bhROO N6RMLTYFAVu8YOWJnFz0e3Y5VVpOg30SyzFFqjgzwd45hSb4MFPoUDTc3b0t4YwoO8by 8rUQ== X-Gm-Message-State: ACrzQf0mZhlrB0cFL9RhM4H27wSyIRbvV1iQAtRsUyTkbA8fNEAqVkkc aKnuSryZiClOuMxF2hTz8ec2jKkhOrRADi8P X-Google-Smtp-Source: AMsMyM5qBnDs3ddkapY+cTfNKpoaOZ2wP8FsnZEikgqmb8dRxqe+PgL2GHOINZvwL2fB/2azHUBpjA== X-Received: by 2002:a17:902:724c:b0:177:5080:cbeb with SMTP id c12-20020a170902724c00b001775080cbebmr3336852pll.67.1665118371960; Thu, 06 Oct 2022 21:52:51 -0700 (PDT) Received: from localhost (76-210-143-223.lightspeed.sntcca.sbcglobal.net. [76.210.143.223]) by smtp.gmail.com with ESMTPSA id 77-20020a621450000000b0056281da3bcbsm532535pfu.149.2022.10.06.21.52.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Oct 2022 21:52:51 -0700 (PDT) Date: Thu, 06 Oct 2022 21:52:51 -0700 (PDT) X-Google-Original-Date: Thu, 06 Oct 2022 21:52:48 PDT (-0700) Subject: Re: [PATCH] PR middle-end/88345: Honor -falign-functions=N even optimized for size. In-Reply-To: <20221007040325.21276-1-kito.cheng@sifive.com> CC: gcc-patches@gcc.gnu.org, Kito Cheng , msebor@gcc.gnu.org, jakub@gcc.gnu.org, rguenth@gcc.gnu.org, koen.zandberg@inria.fr, andy.chiu@sifive.com, guoren@kernel.org, greentime.hu@sifive.com, monk.chiang@sifive.com From: Palmer Dabbelt To: kito.cheng@sifive.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=unavailable 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, 06 Oct 2022 21:03:25 PDT (-0700), kito.cheng@sifive.com wrote: > From: Monk Chiang > > Currnetly setting of -falign-functions=N will be ignored if the function > is optimized for size or marked as cold function. > > However function alignment requirement is needed even optimized for > size in some situations, RISC-V target is an example, RISC-V kernel implement > patchable function that require function must be align to at least 4 bytes for > atomicly patch the function prologue. > > Here is 4 way to fix that: > 1. Fix -falign-functions=N, let it work as expect on -Os and all cold > functions, which is this patch. > 2. Force align to 4 byte if -fpatchable-function-entry is given by adjust > RISC-V's FUNCTION_BOUNDARY. > 3. Adjust RISC-V's FUNCTION_BOUNDARY to let it honor to -falign-functions=N. > 4. Adding a -malign-functions=N for RISC-V...which x86 already deprecated that. > > And this patch is the first approach. > > gcc/ChangeLog: > > PR middle-end/88345 > * varasm.cc (assemble_start_function): Adjust function align > even optimized for size. > * doc/invoke.texi (Os): Remove -falign-functions= from the exclusion > list of -Os. > > gcc/testsuite/ChangeLog: > > PR middle-end/88345 > * gcc.target/i386/pr88345-1.c: New. > * gcc.target/i386/pr88345-2.c: Ditto. > * gcc.target/riscv/pr88345-1.c: Ditto. > * gcc.target/riscv/pr88345-2.c: Ditto. > --- > gcc/doc/invoke.texi | 2 +- > gcc/testsuite/gcc.target/i386/pr88345-1.c | 5 +++++ > gcc/testsuite/gcc.target/i386/pr88345-2.c | 5 +++++ > gcc/testsuite/gcc.target/riscv/pr88345-1.c | 5 +++++ > gcc/testsuite/gcc.target/riscv/pr88345-2.c | 5 +++++ > gcc/varasm.cc | 3 +-- > 6 files changed, 22 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr88345-1.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr88345-2.c > create mode 100644 gcc/testsuite/gcc.target/riscv/pr88345-1.c > create mode 100644 gcc/testsuite/gcc.target/riscv/pr88345-2.c > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index a2b0b9636f0..acf98c68825 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -11381,7 +11381,7 @@ results. This is the default. > Optimize for size. @option{-Os} enables all @option{-O2} optimizations > except those that often increase code size: > > -@gccoptlist{-falign-functions -falign-jumps @gol > +@gccoptlist{-falign-jumps @gol > -falign-labels -falign-loops @gol > -fprefetch-loop-arrays -freorder-blocks-algorithm=stc} > > diff --git a/gcc/testsuite/gcc.target/i386/pr88345-1.c b/gcc/testsuite/gcc.target/i386/pr88345-1.c > new file mode 100644 > index 00000000000..226bb9ffc82 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr88345-1.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-options "-falign-functions=128" } */ > +/* { dg-final { scan-assembler-times "\.p2align\t7" 1 } } */ > + > +__attribute__((__cold__)) void a() {} > diff --git a/gcc/testsuite/gcc.target/i386/pr88345-2.c b/gcc/testsuite/gcc.target/i386/pr88345-2.c > new file mode 100644 > index 00000000000..a7fc3b162dd > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr88345-2.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-options "-falign-functions=128 -Os" } */ > +/* { dg-final { scan-assembler-times "\.p2align\t7" 1 } } */ > + > +void a() {} > diff --git a/gcc/testsuite/gcc.target/riscv/pr88345-1.c b/gcc/testsuite/gcc.target/riscv/pr88345-1.c > new file mode 100644 > index 00000000000..7d5afe683eb > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/pr88345-1.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-options "-falign-functions=128" } */ > +/* { dg-final { scan-assembler-times "\.align\t7" 1 } } */ > + > +__attribute__((__cold__)) void a() {} > diff --git a/gcc/testsuite/gcc.target/riscv/pr88345-2.c b/gcc/testsuite/gcc.target/riscv/pr88345-2.c > new file mode 100644 > index 00000000000..d4fc89d86d8 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/pr88345-2.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > +/* { dg-options "-falign-functions=128 -Os" } */ > +/* { dg-final { scan-assembler-times "\.align\t7" 1 } } */ > + > +void a() {} > diff --git a/gcc/varasm.cc b/gcc/varasm.cc > index 423f3f91af8..4001648b214 100644 > --- a/gcc/varasm.cc > +++ b/gcc/varasm.cc > @@ -1923,8 +1923,7 @@ assemble_start_function (tree decl, const char *fnname) > Note that we still need to align to DECL_ALIGN, as above, > because ASM_OUTPUT_MAX_SKIP_ALIGN might not do any alignment at all. */ > if (! DECL_USER_ALIGN (decl) > - && align_functions.levels[0].log > align > - && optimize_function_for_speed_p (cfun)) > + && align_functions.levels[0].log > align) > { > #ifdef ASM_OUTPUT_MAX_SKIP_ALIGN > int align_log = align_functions.levels[0].log; Reviewed-by: Palmer Dabbelt Though I'm not a global reviewer, so not sure how much that helps...