From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by sourceware.org (Postfix) with ESMTPS id B1026386C5BD for ; Fri, 18 Aug 2023 23:08:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B1026386C5BD Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pf1-x436.google.com with SMTP id d2e1a72fcca58-688731c6331so1198255b3a.3 for ; Fri, 18 Aug 2023 16:08:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1692400097; x=1693004897; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=YWpyCElHROQ3sKhY6k8K6tlqUsHb+RLGID18J98rDZE=; b=C3AvM2+ZNZWgSVKX9YrbJds5VasVLOtJxugZlUXzFtDPZ0dm3gPJCC1wHv4ewjVK5q 1NQJQR8R/imJgCdu62Mz4CKX4xO9kzxfM9JUDvyN+k0fEqEe3ZWednvXjzKVa29NMgLQ LZ3rePfydAg5uVhP4Ejn17+hP4Ac0VLoNExlaht0gRCIOKeEQG2r2a1ZKUHFXaQLswTv 336cM1K4hckBaMtD29yFIRzX01GAiGbZImixN4a7UI1ic4Z8KbgEnO71L1m0txdci2Fb jkw5TVFbAiWU9YrqAVl6BShw1EmSkQhGmSrJcdR7sidQ/GI+/oyKk7P/23GeoopPXedF 2YAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692400097; x=1693004897; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=YWpyCElHROQ3sKhY6k8K6tlqUsHb+RLGID18J98rDZE=; b=cbRRV82tpcE9fLSSx4Ke7Gun7TmiGB1MV+uXEk0435TVBR9ZR6G6gdix6+FJLSOI/o Ag7Xuqp0XsrZhh+OpRtOR/0iw6CpdqIn6dT6Yvh+srZfur86k4iryBTUFdjN/SjJ/Ck2 KjwWb0XRlunFJ36/MRfzUzv0p76k99IwGvtkTBc7nWJeBt2UTfgamqNs6vinaMl3I6P3 wqKjHlSNpaOWN/zrZzGlMqvU0X94vrQicCNNzPP6DdKvufNEyC5xIOREq5d3HJUaI4o+ LCgtE0SKt+VZJNCp1OG1GDgaHjdIAwB7ss8y6kfaEHd4XPHsCBU0uAI6wMZ1hxoPnREs LeMw== X-Gm-Message-State: AOJu0Yx/vNjQwI1eanO9pXkIpYP8E1sCnNjE9If49m8mNqMyxiZdXMG/ 684mkAW2S45rxdV71bOuLUw= X-Google-Smtp-Source: AGHT+IHnDu/0RLlyIe53JyLGJ+4thvxpK4wTq3T5cET7yq5ZPvw/7j0jlF/XHTktvwdu0r1HuJqdOg== X-Received: by 2002:a05:6a00:3911:b0:686:fd66:a41c with SMTP id fh17-20020a056a00391100b00686fd66a41cmr655536pfb.17.1692400096545; Fri, 18 Aug 2023 16:08:16 -0700 (PDT) Received: from [172.31.0.109] ([136.36.130.248]) by smtp.gmail.com with ESMTPSA id i2-20020aa78b42000000b0068725ff9befsm2009743pfd.207.2023.08.18.16.08.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 18 Aug 2023 16:08:16 -0700 (PDT) Message-ID: <04b8f9ee-31f6-d699-7acc-acfa1b9102e5@gmail.com> Date: Fri, 18 Aug 2023 17:08:14 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH] RISC-V: Enable pressure-aware scheduling by default. Content-Language: en-US To: Robin Dapp , gcc-patches , palmer , Kito Cheng , "juzhe.zhong@rivai.ai" , vineetg@rivosinc.com References: <6c8b8a16-bbf9-b697-0f4c-26a838fb5665@gmail.com> From: Jeff Law In-Reply-To: <6c8b8a16-bbf9-b697-0f4c-26a838fb5665@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_MANYTO,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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 8/18/23 07:57, Robin Dapp wrote: > Hi, > > this patch enables pressure-aware scheduling for riscv. There have been > various requests for it so I figured I'd just go ahead and send > the patch. Thanks. Your timing is good, I was pondering nominating a victim to pick this up next week ;-) > > There is some slight regression in code quality for a number of > vector tests where we spill more due to different instructions order. > The ones I looked at were a mix of bad luck and/or brittle tests. > Comparing the size of the generated assembly or the number of vsetvls > for SPECint also didn't show any immediate benefit but that's obviously > not a very fine-grained analysis. Yea. In fact I wouldn't really expect significant changes other than those key loops in x264. > > As cost and scheduling models mature I expect the situation to improve > and for now I think it's generally favorable to enable pressure-aware > scheduling so we can work with it rather than trying to find every > possible problem in advance. Any other opinions on that? Agreed. I wouldn't be surprised if largely turns into a set-and-forget. > > Regards > Robin > > This patch enables register -fsched-pressure by default and sets > the algorithm to "model". As with other backends, this helps > reduce unnecessary spills. > > gcc/ChangeLog: > > * common/config/riscv/riscv-common.cc: Add -fsched-pressure. > * config/riscv/riscv.cc (riscv_option_override): Set sched > pressure algorithm. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/rvv/base/narrow_constraint-1.c: Add > -fno-sched-pressure. > * gcc.target/riscv/rvv/base/narrow_constraint-17.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-18.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-19.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-20.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-21.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-22.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-23.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-24.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-25.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-26.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-27.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-28.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-29.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-30.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-31.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-4.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-5.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-8.c: Ditto. > * gcc.target/riscv/rvv/base/narrow_constraint-9.c: Ditto. > * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-10.c: Ditto. > * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-11.c: Ditto. > * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-12.c: Ditto. > * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-3.c: Ditto. > * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-9.c: Ditto. OK. Just one nit below. > > diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc > index 4737dcd44a1..59848b21162 100644 > --- a/gcc/common/config/riscv/riscv-common.cc > +++ b/gcc/common/config/riscv/riscv-common.cc > @@ -2017,9 +2017,11 @@ static const struct default_options riscv_option_optimization_table[] = > { > { OPT_LEVELS_1_PLUS, OPT_fsection_anchors, NULL, 1 }, > { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 }, > + { OPT_LEVELS_1_PLUS, OPT_fsched_pressure, NULL, 1 }, > #if TARGET_DEFAULT_ASYNC_UNWIND_TABLES == 1 > { OPT_LEVELS_ALL, OPT_fasynchronous_unwind_tables, NULL, 1 }, > { OPT_LEVELS_ALL, OPT_funwind_tables, NULL, 1}, > + /* Enable -fsched-pressure by default when optimizing. */ > #endif > { OPT_LEVELS_NONE, 0, NULL, 0 } > }; Shouldn't the comment move up to before the OPT_fsched_pressure line? > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 49062bef9fc..96c5362d2fd 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -65,6 +65,7 @@ along with GCC; see the file COPYING3. If not see > #include "cfgloop.h" > #include "cfgrtl.h" > #include "sel-sched.h" > +#include "sched-int.h" > #include "fold-const.h" > #include "gimple-iterator.h" > #include "gimple-expr.h" > @@ -7095,6 +7096,10 @@ riscv_option_override (void) > sorry ( > "Current RISC-V GCC cannot support VLEN greater than 4096bit for 'V' Extension"); > > + SET_OPTION_IF_UNSET (&global_options, &global_options_set, > + param_sched_pressure_algorithm, > + SCHED_PRESSURE_MODEL); > + FWIW, I tried both variants of the pressure model. They seemed roughly on-par with each other across specint. Jeff