From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) by sourceware.org (Postfix) with ESMTPS id 31DD7385800C for ; Fri, 19 Feb 2021 05:54:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 31DD7385800C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=debian.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=wzssyqa@gmail.com Received: by mail-qt1-f170.google.com with SMTP id v3so3236055qtw.4 for ; Thu, 18 Feb 2021 21:54:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=6quU4FeIlk18MyjXZn4+QSWgCRktOM1WbeqwgSrbPHE=; b=HY8kCYlTqhrTnJhgbfQtID7U99MadMgYFlprcggEIgUrmyRqDOZV7C20v+RAwQCuhx XBO6QAcn94a6iuz2/4/d1XgkI4VjPQV7rEiORUQvn9+FCk9VVf01rl4ZKjFmJyM4z8lI DFZLyc7nEOOEUlCxk1KWlMenAF4VCLfC6C8iCIWtX3Op8yntDX8/Iff/ie+qfJuwTndL TWeVhJfifpSu/JffJJWDzXNWnM+6aJu6mRxGmmJbcUpN5/azOsrEnhlYVsjM4t7qImed pKtbOUm3iLzhBbTDrix8AET4+GrRf2u37FqZcpbjVGNVTPQwM7F3F5QWatv8J9B0hPcy 2sRQ== X-Gm-Message-State: AOAM5320ztMTHrykLQjpSByr+OqnZFrSSQutXKg9zAre+K2XBFeYWN32 T/9VbD6lDfP/uO05yPQDLLD8OSnbzq6hv5XBsG8= X-Google-Smtp-Source: ABdhPJyxe1f8A7mTB1nGFb7UjZSySlYX9ovYr3QqCFlaRiOivw1xbpI52LPVUkTC7rv7xOv6o2wtnXkP4VDyQj/JGU8= X-Received: by 2002:ac8:100b:: with SMTP id z11mr7674563qti.60.1613714053342; Thu, 18 Feb 2021 21:54:13 -0800 (PST) MIME-Version: 1.0 References: <20210205095338.28231-1-yunqiang.su@cipunited.com> <20210205095338.28231-2-yunqiang.su@cipunited.com> <0d221863-8649-5747-a9a7-802da76d6062@redhat.com> In-Reply-To: From: YunQiang Su Date: Fri, 19 Feb 2021 13:55:49 +0800 Message-ID: Subject: Re: [PATCH 2/3] MIPS: add builtime option for -mcompact-branches To: "Maciej W. Rozycki" Cc: Jeff Law , YunQiang Su , gcc-patches@gcc.gnu.org, Jiaxun Yang Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Feb 2021 05:54:16 -0000 Maciej W. Rozycki =E4=BA=8E2021=E5=B9=B42=E6=9C=8817=E6= =97=A5=E5=91=A8=E4=B8=89 =E4=B8=8A=E5=8D=883:45=E5=86=99=E9=81=93=EF=BC=9A > > On Tue, 16 Feb 2021, Jeff Law via Gcc-patches wrote: > > > > diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi > > > index 4c38244ae58..6b9520569ba 100644 > > > --- a/gcc/doc/install.texi > > > +++ b/gcc/doc/install.texi > > > @@ -1464,6 +1464,29 @@ systems that support conditional traps). > > > Division by zero checks use the break instruction. > > > @end table > > > > > > +@item --with-compact-branches=3D@var{policy} > > > +Specify how the compiler should generate code for checking for > > > +division by zero. This option is only supported on the MIPS target. > > > +The possibilities for @var{type} are: > > Is this really correct -- I would expect that the change affects > > branches in general, not just checks for division by zero. > > I wonder if we need to multiply the options here at all. The original > change: was > discussed here: > in this respect: > > On Mon, 17 Aug 2015, Matthew Fortune wrote: > > > > > Compact branches are used based on a branch policy. The polices are= : > > > > > > > > never: Only use delay slot branches > > > > optimal: Do whatever is best for the current architecture. This wi= ll > > > > generally mean that delay slot branches will be used if th= e delay > > > > slot gets filled but otherwise a compact branch will be us= ed. A > > > > special case here is that JAL and J will not be used in R6= code > > > > regardless of whether the delay slot could be filled. > > > > always: Never emit a delay slot form of a branch if a compact form = exists. > > > > This policy cannot apply 100% as FP branches (and MSA branc= hes when > > > > committed) only have delay slot forms. > > > > > > > > These user choices are combined with the features available in the = chosen > > > > architecture and, in particular, the optimal form will get handled = like > > > > 'never' when there are no compact branches available and will get h= andled > > > > like 'always' when there are no delay slot branches available. > > > > > > > > > > Why did you choose to make this a user-selectable option? Why not al= ways generated > > > optimal? > > > I don't have a strong opinion about it, but the options seem to compl= icate things and I'm > > > interested in your rationale. > > > > This is down to micro-architecture decisions that different implementer= s may make. > > Honestly, I have not absorbed all of the rationale behind choosing one = form over > > the other but our arch team have made enough comments about this to mea= n the support > > in the compiler is worth the extra bit of effort. I can attempt a write= -up of some > > of the pipeline possibilities if you would like more detail but I'd pro= bably have to > > refresh my mind on this with our hardware teams. > > My understanding therefore is that the original assumption that `optimal= ' > will serve people best is no longer true. > I guess that `optimal' can still produce the best performance, while the delay slot make MIPS quite differnent with other architectures. And the hardware engineers seems hate it also. And we expect that MIPS can have as few as possible differnece delta with other major architectures, to ultily all of new framworks of community. > First, I think it would be good if we knew why. I find proliferating > variants of defaults, especially for the less obvious cases, will cause > user confusion as one won't know what code model to expect, especially as > (please correct me if I am wrong) we don't actually provide a way to dump > the compiler's overridable configuration defaults. > So, should we provide a predefined macro for it? > Second, I wonder if it makes sense to just keep things simple, and rathe= r > than adding `prefer' (to stand for "`always' if available"), and possibly > `avoid' (to stand for "`never' if available"), whether we shouldn't just > relax the checks for `always' and `never', and let them through whether > the architecture selected provides for the option chosen or not. > relax the `always' is what I would like to do first. But I was afread to break some complatiable. > Please note that in the discussion quoted Catherine has already raised a > concern I agree with of adding a complication here, and now we would > complicate it even further by not only adding a fourth choice, but anothe= r > overridable configuration default as well. I am still concern about whether we should just set the `always' as default= . My short team plan is to set it default for Debian r6 Port. So, at least, I wish that we can provide a buildtime option for other need. > > Maciej