From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id 370403858C56 for ; Tue, 26 Jul 2022 21:04:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 370403858C56 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-Transfer-Encoding:Content-Type: MIME-Version:Message-ID:Date:Subject:In-Reply-To:References:Cc:To:From:Sender :Reply-To: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=uZ0dCDDHNzBGbg/W2Ms1BwFQyLunXQq5cLJgN5hXLKA=; b=eFSShx6/L6aPoUyNog/rqWQIdx /4z5FMtAlTcDXghWzYyuHJl1CcUvkXa4N/qXIGOke+AocfEW5nQamvtC39mYSL8c9GkaOkaMRh27S IWkS7s1X6ezJfu9MBJp5I39FD7rifRhxHbthLEB2sOPbbEiwBWiVRQPizJPMli3C/+03b00+C4yjE ZFLD1RImPZ2KgrhUomKF2WYcLI2T7ZevJfrJWX3i2z0gh2X6RvrlYD4pk5tigY0dK/1uqamNcOLci 63TSejtPh5yqCXGKeRvJFN+dCWjwU3LxJKBDcddSEaa1gIuFoXfpsmVqdN2DYQePgQWXnQol4Osov vSZmhTTg==; Received: from host81-156-58-95.range81-156.btcentralplus.com ([81.156.58.95]:54148 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 1oGRjU-0001Pg-Do; Tue, 26 Jul 2022 17:04:48 -0400 From: "Roger Sayle" To: "'Segher Boessenkool'" Cc: References: <003601d89245$a86f8830$f94e9890$@nextmovesoftware.com> <20220707223833.GA25951@gate.crashing.org> <00fa01d8a0e9$0efdfc60$2cf9f520$@nextmovesoftware.com> <20220726174451.GA25951@gate.crashing.org> In-Reply-To: <20220726174451.GA25951@gate.crashing.org> Subject: RE: [PATCH] Add new target hook: simplify_modecc_const. Date: Tue, 26 Jul 2022 22:04:45 +0100 Message-ID: <01f501d8a133$56fee8e0$04fcbaa0$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQKM31p2pOYqjcFJ2r+wKuC+8ENZTgCpq+KpAm2gSXsBzF2qA6wBd9gA 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=-6.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, 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: , X-List-Received-Date: Tue, 26 Jul 2022 21:04:53 -0000 Hi Segher, It's very important to distinguish the invariants that exist for the RTL data structures as held in memory (rtx), vs. the use of "enum rtx_code"s, "machine_mode"s and operands in the various processing functions of the middle-end. Yes, it's very true that RTL integer constants don't specify a mode (are VOIDmode), so therefore operations like ZERO_EXTEND or EQ don't make sense with all constant operands. This is (one reason) why constant-only operands are disallowed from RTL (data structures), and why in APIs that perform/simplify these operations, the original operand mode (of the const_int(s)) must be/is always passed as a parameter. Hence, for say simplify_const_binary_operation, op0 and op1 can both be const_int, as the mode argument specifies the mode of the "code" operation. Likewise, in simplify_relational_operation, both op0 and op1 may be CONST_INT as "cmp_mode" explicitly specifies the mode that the operation is performed in and "mode" specifies the mode of the result. Your comment that "comparing two integer constants is invalid RTL *in all contexts*" is a serious misunderstanding of what's going on. At no point is a RTL rtx node ever allocated with two integer constant operands. RTL simplification is for hypothetical "what if" transformations (just like try_combine calls recog with RTL that may not be real instructions), and these simplifcations are even sometimes required to preserve the documented RTL invariants. Comparisons of two integers must be simplified to true/false precisely to ensure that they never appear in an actual COMPARE node. I worry this fundamental misunderstanding is the same issue that has been holding up understanding/approving a previous patch: https://gcc.gnu.org/pipermail/gcc-patches/2021-September/578848.html For a related bug, consider PR rtl-optimization/67382, that's assigned to you in bugzilla. In this case, the RTL optimizers know that both operands to a COMPARE are integer constants (both -2), yet the compiler still performs a run-time comparison and conditional jump: movl $-2, %eax movl %eax, 12(%rsp) cmpl $-2, %eax je .L1 Failing to optimize/consider a comparison between two integer constants *in any context* just leads to poor code. Hopefully, this clears up that the documented constraints on RTL rtx aren't exactly the same as the constraints on the use of rtx_codes in simplify-rtx's functional APIs. So simplify_subreg really gets called on operands that are neither REG nor MEM, as this is unrelated to what the documentation of the SUBREG rtx specifies. If you don't believe that op0 and op1 can ever both be const_int in this function, perhaps consider it harmless dead code and humor me. Thanks in advance, Roger -- > -----Original Message----- > From: Segher Boessenkool > Sent: 26 July 2022 18:45 > To: Roger Sayle > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] Add new target hook: simplify_modecc_const. > > Hi! > > On Tue, Jul 26, 2022 at 01:13:02PM +0100, Roger Sayle wrote: > > 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). > > This is invalid RTL. What would (set (reg:CC) (const_int 0)) mean, for example? > If this was valid it would make most existing code using CC modes do essentially > random things :-( > > The documentation (in tm.texi, "Condition Code") says > Alternatively, you can use @code{BImode} if the comparison operator is > specified already in the compare instruction. In this case, you are not > interested in most macros in this section. > > > 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. > > Hrm, I wonder how other targets do this. > > On Power we have a separate hard register for the carry flag of course (it is a > separate bit in the hardware as well, XER[CA]). > > On Arm there is arm_carry_operation (as well as arm_borrow_operation). > > Aarch64 directly uses > (define_expand "add3_carryin" > [(set (match_operand:GPI 0 "register_operand") > (plus:GPI > (plus:GPI > (ltu:GPI (reg:CC_C CC_REGNUM) (const_int 0)) > (match_operand:GPI 1 "aarch64_reg_or_zero")) > (match_operand:GPI 2 "aarch64_reg_or_zero")))] > "" > "" > ) > (CC_Cmode means only the C bit is validly set). > > s390 does similar. sparc does similar. > > > 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] > > There are eight condition register fields which can be used interchangeably > (some insns only write to CR0, CR1, or CR6). The meaning of the four bits in a > field depends on the instruction that set them. For integer comparisons bit 3 > does not mean anything to do with a > comparison: instead, it is a copy of the XER[SO] bit ("summary overflow"). The > rs6000 backend does not currently model this (we do not model the overflow > instructions at all!) > > > 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 > > crand, crnand, cror, crxor, crnor, creqv, crandc, crorc insns, or the extended > mnemonics crmove, crclr, crnot, crset, yes. All these for setting single bits; > there also is mcrf to copy all four bits of a CR field to another. > > > 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. > > And yet we do. See for example @cceq_rev_compare_ which > implements crnot. > > > + /* 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); > > Comparing two integer constants is invalid RTL *in all contexts*. The items > compared do not have a mode! From rtl.texi: > A @code{compare} specifying two @code{VOIDmode} constants is not valid > since there is no way to know in what mode the comparison is to be > performed; the comparison must either be folded during the compilation > or the first operand must be loaded into a register while its mode is > still known. > > > Segher