Message ID | 003601d89245$a86f8830$f94e9890$@nextmovesoftware.com |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E39A3385C422 for <patchwork@sourceware.org>; Thu, 7 Jul 2022 21:08:26 +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 414BF3858CDA for <gcc-patches@gcc.gnu.org>; Thu, 7 Jul 2022 21:08:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 414BF3858CDA 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: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:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=pK33qm8qcN5q1eM2681RxH0gB/cPXeZarWyiMNSX8Fg=; b=OQ5bhVHTL/Ifxl7kfo8+u7DStd n+9bW4qQnlUSnfM+gl8eydSgNBJGnnCkVadt6roVKKl2cD2TDkYAGBuvbsj3dtjmqlqQ93ubN4kUx vJNR6WTrR3Qjg8fWAJNw68vimWJdvldUq5241WkcrqYSRjG/FXKuOwLopomNUKKcdWavyM3njxYlK 3VxVnwJIbC6WRRFOaGVXv7DsqlJObxmYJjH3N+9EiyBuGhH9FNgfkfGuLCglRxtvPKodfR+E514j6 HSoJs7Y5IXZlR5KdMANB6/l8Vkua7nM/d4Zpbs52q0dkM2tOgHFqN968TdyYtKLKIFKGGKGwKealn ScEjRCSw==; Received: from host86-130-134-60.range86-130.btcentralplus.com ([86.130.134.60]:59686 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 <roger@nextmovesoftware.com>) id 1o9YjJ-0003nT-9a; Thu, 07 Jul 2022 17:08:09 -0400 From: "Roger Sayle" <roger@nextmovesoftware.com> To: <gcc-patches@gcc.gnu.org> Subject: [PATCH] Be careful with MODE_CC in simplify_const_relational_operation. Date: Thu, 7 Jul 2022 22:08:04 +0100 Message-ID: <003601d89245$a86f8830$f94e9890$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0037_01D8924E.0A366130" X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdiSRMIyPq4dwSlMTduqSIxNckhQxQ== 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=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Cc: 'Segher Boessenkool' <segher@kernel.crashing.org> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
Be careful with MODE_CC in simplify_const_relational_operation.
|
|
Commit Message
Roger Sayle
July 7, 2022, 9:08 p.m. UTC
I think it's fair to describe RTL's representation of condition flags using MODE_CC as a little counter-intuitive. 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. 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. 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]. 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}, with no new failures, and in combination with a i386 backend patch (that introduces support for x86's stc and clc instructions) where it avoids failures. However, I'm submitting this middle-end piece independently, to confirm that maintainers/reviewers are happy with the approach, and also to check there are no issues on other platforms, before building upon this infrastructure. Thoughts? Ok for mainline? 2022-07-07 Roger Sayle <roger@nextmovesoftware.com> gcc/ChangeLog * simplify-rtx.cc (simplify_const_relational_operation): Handle case where both operands of a MODE_CC comparison have been simplified to constant integers. Thanks in advance, Roger --
Comments
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/simplify-rtx.cc b/gcc/simplify-rtx.cc index fa20665..73ec5c7 100644 --- a/gcc/simplify-rtx.cc +++ b/gcc/simplify-rtx.cc @@ -6026,6 +6026,18 @@ simplify_const_relational_operation (enum rtx_code code, return 0; } + /* 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. */ if (GET_MODE_CLASS (GET_MODE (op0)) == MODE_CC)