From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id DB6433858D32 for ; Tue, 26 Jul 2022 17:46:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DB6433858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 26QHj78E012927; Tue, 26 Jul 2022 12:45:07 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 26QHj173012919; Tue, 26 Jul 2022 12:45:01 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Tue, 26 Jul 2022 12:44:51 -0500 From: Segher Boessenkool To: Roger Sayle Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Add new target hook: simplify_modecc_const. Message-ID: <20220726174451.GA25951@gate.crashing.org> References: <003601d89245$a86f8830$f94e9890$@nextmovesoftware.com> <20220707223833.GA25951@gate.crashing.org> <00fa01d8a0e9$0efdfc60$2cf9f520$@nextmovesoftware.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <00fa01d8a0e9$0efdfc60$2cf9f520$@nextmovesoftware.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no 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 17:46:11 -0000 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