From patchwork Tue Jul 26 12:13:02 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 1660782 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nextmovesoftware.com header.i=@nextmovesoftware.com header.a=rsa-sha256 header.s=default header.b=OmCIH13Q; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (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 bilbo.ozlabs.org (Postfix) with ESMTPS id 4LsbR65cTFz9sFr for ; Tue, 26 Jul 2022 22:13:21 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1EFEA385842B for ; Tue, 26 Jul 2022 12:13:18 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id 5A72B3858400 for ; Tue, 26 Jul 2022 12:13:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5A72B3858400 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Type:MIME-Version:Message-ID: Date:Subject:In-Reply-To:References:Cc:To:From:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=Ku0vZWeXHk2o8ojn1P+PbPzPTORCyVUdJa/JRH8xQZ4=; b=OmCIH13QAQk9pQvhGZ5R1/jo1A Zedqo5/LgumY+Hf2r/CpJCTfDBZ6ACMSCotVo/bOylr5zY4XDb8Zdhsab+khMvU+xJnLtjqszM8aY yqQVjTZt21dvf93aghr+LrEbBE/5uYD4gMjnWOfUZwwHCrS+6ocid7cyGyqJpcpG2tMUELTAgkjUE rmBqgdOW6Y9A57SRBqR8rkVHEi0tCUdBnqzBGO7aeoG4Gk88iPqBx+vwrV3OkQJG99/znpK7ZCnaG IG8PC75vuGWKAZbLGdh9tKHlYakM6TQoCxvo5M2G7uNdRyeKRFotGl4ZeJ9OHAQHvAd1b9ZecZbPF vz/IWoqw==; Received: from host86-167-159-17.range86-167.btcentralplus.com ([86.167.159.17]:63228 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1oGJQu-00024z-FU; Tue, 26 Jul 2022 08:13:04 -0400 From: "Roger Sayle" To: "'Segher Boessenkool'" References: <003601d89245$a86f8830$f94e9890$@nextmovesoftware.com> <20220707223833.GA25951@gate.crashing.org> In-Reply-To: <20220707223833.GA25951@gate.crashing.org> Subject: [PATCH] Add new target hook: simplify_modecc_const. Date: Tue, 26 Jul 2022 13:13:02 +0100 Message-ID: <00fa01d8a0e9$0efdfc60$2cf9f520$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: Adig5j+5uPv68wE+QNqqfMmbUU+2tQ== Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, 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 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: , Cc: gcc-patches@gcc.gnu.org Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" This patch is a major revision of the patch I originally proposed here: https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598040.html The primary motivation of this patch is to avoid incorrect optimization of MODE_CC comparisons in simplify_const_relational_operation when/if a backend represents the (known) contents of a MODE_CC register using a CONST_INT. In such cases, the RTL optimizers don't know the semantics of this integer value, so shouldn't change anything (i.e. should return NULL_RTX from simplify_const_relational_operation). The secondary motivation is that by introducing a new target hook, called simplify_modecc_const, the backend can (optionally) encode and interpret a target dependent encoding of MODE_CC registers. The worked example provided with this patch is to allow the i386 backend to explicitly model the carry flag (MODE_CCC) using 1 to indicate that the carry flag is set, and 0 to indicate the carry flag is clear. This allows the instructions stc (set carry flag), clc (clear carry flag) and cmc (complement carry flag) to be represented in RTL. However an even better example would be the rs6000 backend, where this patch/target hook would allow improved modelling of the condition register CR. The powerpc's comparison instructions set fields/bits in the CR register [where bit 0 indicates less than, bit 1 greater than, bit 2 equal to and bit3 overflow] analogous to x86's flags register [containing bits for carry, zero, overflow, parity etc.]. These fields can be manipulated directly using crset (aka creqv) and crclr (aka crxor) instructions and even transferred from general purpose registers using mtcr. However, without a patch like this, it's impossible to safely model/represent these instructions in rs6000.md. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check, both with and without --target_board=unix{-m32}, and both with and without a patch to add stc, clc and cmc support to the x86 backend. I'll resubmit the x86 target pieces again with that follow-up backend patch, so for now I'm only looking for approval of the middle-end infrastructure pieces. The x86 hunks below are provided as context/documentation for how this hook could/should be used (but I wouldn't object to pre-approval of those bits by Uros). Ok for mainline? 2022-07-26 Roger Sayle gcc/ChangeLog * target.def (simplify_modecc_const): New target hook. * doc/tm.texi (TARGET_SIMPLIFY_MODECC_CONST): Document here. * doc/tm.texi.in (TARGET_SIMPLIFY_MODECC_CONST): Locate @hook here. * hooks.cc (hook_rtx_mode_int_rtx_null): Define default hook here. * hooks.h (hook_rtx_mode_int_rtx_null): Prototype here. * simplify-rtx.c (simplify_const_relational_operation): Avoid mis-optimizing MODE_CC comparisons by calling new target hook. * config/i386.cc (ix86_simplify_modecc_const): Implement new target hook, supporting interpreting MODE_CCC values as the x86 carry flag. (TARGET_SIMPLIFY_MODECC_CONST): Define as ix86_simplify_modecc_const. Thanks in advance, Roger --- > -----Original Message----- > From: Segher Boessenkool > Sent: 07 July 2022 23:39 > To: Roger Sayle > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] Be careful with MODE_CC in > simplify_const_relational_operation. > > Hi! > > On Thu, Jul 07, 2022 at 10:08:04PM +0100, Roger Sayle wrote: > > I think it's fair to describe RTL's representation of condition flags > > using MODE_CC as a little counter-intuitive. > > "A little challenging", and you should see that as a good thing, as a puzzle to > crack :-) > > > For example, the i386 > > backend represents the carry flag (in adc instructions) using RTL of > > the form "(ltu:SI (reg:CCC) (const_int 0))", where great care needs to > > be taken not to treat this like a normal RTX expression, after all LTU > > (less-than-unsigned) against const0_rtx would normally always be > > false. > > A comparison of a MODE_CC thing against 0 means the result of a > *previous* comparison (or other cc setter) is looked at. Usually it simply looks > at some condition bits in a flags register. It does not do any actual comparison: > that has been done before (if at all even). > > > Hence, MODE_CC comparisons need to be treated with caution, and > > simplify_const_relational_operation returns early (to avoid > > problems) when GET_MODE_CLASS (GET_MODE (op0)) == MODE_CC. > > Not just to avoid problems: there simply isn't enough information to do a > correct job. > > > However, consider the (currently) hypothetical situation, where the > > RTL optimizers determine that a previous instruction unconditionally > > sets or clears the carry flag, and this gets propagated by combine > > into the above expression, we'd end up with something that looks like > > (ltu:SI (const_int 1) (const_int 0)), which doesn't mean what it says. > > Fortunately, simplify_const_relational_operation is passed the > > original mode of the comparison (cmp_mode, the original mode of op0) > > which can be checked for MODE_CC, even when op0 is now VOIDmode > > (const_int) after the substitution. Defending against this is clearly > > the right thing to do. > > > > More controversially, rather than just abort > > simplification/optimization in this case, we can use the comparison > > operator to infer/select the semantics of the CC_MODE flag. > > Hopefully, whenever a backend uses LTU, it represents the (set) carry > > flag (and behaves like i386.md), in which case the result of the simplified > expression is the first operand. > > [If there's no standardization of semantics across backends, then we > > should always just return 0; but then miss potential optimizations]. > > On PowerPC, ltu means the result of an unsigned comparison (we have > instructions for that, cmpl[wd][i] mainly) was "smaller than". It does not mean > anything is unsigned smaller than zero. It also has nothing to do with carries, > which are done via a different register (the XER). > > > + /* Handle MODE_CC comparisons that have been simplified to > > + constants. */ > > + if (GET_MODE_CLASS (mode) == MODE_CC > > + && op1 == const0_rtx > > + && CONST_INT_P (op0)) > > + { > > + /* LTU represents the carry flag. */ > > + if (code == LTU) > > + return op0 == const0_rtx ? const0_rtx : const_true_rtx; > > + return 0; > > + } > > + > > /* We can't simplify MODE_CC values since we don't know what the > > actual comparison is. */ > > ^^^ > This comment is 100% true. We cannot simplify any MODE_CC comparison > without having more context. The combiner does have that context when it > tries to combine the CC setter with the CC consumer, for example. > > Do you have some piece of motivating example code? > > > Segher diff --git a/gcc/target.def b/gcc/target.def index 2a7fa68..cd81b4e 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -7107,6 +7107,20 @@ counters are incremented using atomic operations. Targets not supporting\n\ type.", HOST_WIDE_INT, (void), default_gcov_type_size) +DEFHOOK +(simplify_modecc_const, + "On targets that represent the result of a comparison using a\n\ +@code{MODE_CC} flags register, allow the backend to represent a known\n\ +result using a target-dependent integer constant. This helper function\n\ +of @code{simplify_const_relational_operation} attempts to simplify a\n\ +comparison operator @var{code} that is defined by a @code{COMPARE} in mode\n\ +@var{mode}, which is known to be of class @code{MODE_CC}, and is encoded\n\ +as @var{val}, which must be a @code{CONST_INT}. The result must be\n\ +@code{const_true_rtx}, @code{const0_rtx} or @code{NULL_RTX} if no\n\ +simplification is possible.", + rtx, (machine_mode mode, int code, rtx val), + hook_rtx_mode_int_rtx_null) + /* This value represents whether the shadow call stack is implemented on the target platform. */ DEFHOOKPOD diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index b0ea398..5a1231d 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -6631,6 +6631,18 @@ post-reload comparison elimination pass, or the delay slot filler pass, then this value should be set appropriately. @end deftypevr +@deftypefn {Target Hook} rtx TARGET_SIMPLIFY_MODECC_CONST (machine_mode @var{mode}, int @var{code}, rtx @var{val}) +On targets that represent the result of a comparison using a +@code{MODE_CC} flags register, allow the backend to represent a known +result using a target-dependent integer constant. This helper function +of @code{simplify_const_relational_operation} attempts to simplify a +comparison operator @var{code} that is defined by a @code{COMPARE} in mode +@var{mode}, which is known to be of class @code{MODE_CC}, and is encoded +as @var{val}, which must be a @code{CONST_INT}. The result must be +@code{const_true_rtx}, @code{const0_rtx} or @code{NULL_RTX} if no +simplification is possible. +@end deftypefn + @node Costs @section Describing Relative Costs of Operations @cindex costs of instructions diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index f869ddd..8cecde7 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -4397,6 +4397,8 @@ like: @hook TARGET_FLAGS_REGNUM +@hook TARGET_SIMPLIFY_MODECC_CONST + @node Costs @section Describing Relative Costs of Operations @cindex costs of instructions diff --git a/gcc/hooks.cc b/gcc/hooks.cc index b29233f..90be303 100644 --- a/gcc/hooks.cc +++ b/gcc/hooks.cc @@ -401,6 +401,14 @@ hook_rtx_tree_int_null (tree, int) return NULL; } +/* Generic hook that takes a machine mode, an int and an rtx and + returns NULL_RTX. */ +rtx +hook_rtx_mode_int_rtx_null (machine_mode, int, rtx) +{ + return NULL; +} + /* Generic hook that takes a machine mode and returns an unsigned int 0. */ unsigned int hook_uint_mode_0 (machine_mode) diff --git a/gcc/hooks.h b/gcc/hooks.h index 1056e1e..e978817 100644 --- a/gcc/hooks.h +++ b/gcc/hooks.h @@ -123,6 +123,7 @@ extern bool default_can_output_mi_thunk_no_vcall (const_tree, HOST_WIDE_INT, extern rtx hook_rtx_rtx_identity (rtx); extern rtx hook_rtx_rtx_null (rtx); extern rtx hook_rtx_tree_int_null (tree, int); +extern rtx hook_rtx_mode_int_rtx_null (machine_mode, int, rtx); extern char *hook_charptr_void_null (void); extern const char *hook_constcharptr_void_null (void); diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc index fa20665..699e08a 100644 --- a/gcc/simplify-rtx.cc +++ b/gcc/simplify-rtx.cc @@ -6012,6 +6012,13 @@ simplify_const_relational_operation (enum rtx_code code, || (GET_MODE (op0) == VOIDmode && GET_MODE (op1) == VOIDmode)); + /* Handle MODE_CC comparisons that have been simplified to + constants. */ + if (GET_MODE_CLASS (mode) == MODE_CC + && op1 == const0_rtx + && CONST_INT_P (op0)) + return targetm.simplify_modecc_const (mode, (int)code, op0); + /* If op0 is a compare, extract the comparison arguments from it. */ if (GET_CODE (op0) == COMPARE && op1 == const0_rtx) { diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index e03f86d..a73c265 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -15995,6 +15995,31 @@ ix86_cc_modes_compatible (machine_mode m1, machine_mode m2) } } +/* Simplify the results of comparisons with constant operands. + MODE is a MODE_CC, CODE is a comparison operator, and VAL is + a target (and mode) dependent CONST_INT. */ + +rtx +ix86_simplify_modecc_const (machine_mode mode, int code, rtx val) +{ + /* For CCCmode, const0_rtx represents the carry flag is clear, + otherwise the carry flag is set. */ + switch ((enum rtx_code)code) + { + case LTU: + if (mode == E_CCCmode) + return val == const0_rtx ? const0_rtx : const_true_rtx; + break; + case GEU: + if (mode == E_CCCmode) + return val == const0_rtx ? const_true_rtx : const0_rtx; + break; + default: + break; + } + return NULL_RTX; +} + /* Return strategy to use for floating-point. We assume that fcomi is always preferrable where available, since that is also true when looking at size (2 bytes, vs. 3 for fnstsw+sahf and at least 5 for fnstsw+test). */ @@ -24978,6 +25003,9 @@ static bool ix86_libc_has_fast_function (int fcode ATTRIBUTE_UNUSED) #undef TARGET_GEN_MEMSET_SCRATCH_RTX #define TARGET_GEN_MEMSET_SCRATCH_RTX ix86_gen_scratch_sse_rtx +#undef TARGET_SIMPLIFY_MODECC_CONST +#define TARGET_SIMPLIFY_MODECC_CONST ix86_simplify_modecc_const + #if CHECKING_P #undef TARGET_RUN_TARGET_SELFTESTS #define TARGET_RUN_TARGET_SELFTESTS selftest::ix86_run_selftests