From patchwork Fri Feb 3 09:15:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 52418 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp728894wrn; Fri, 3 Feb 2023 01:16:26 -0800 (PST) X-Google-Smtp-Source: AK7set8jugvWCGe72uB72qr6kQNz2Kh17SF9CvihpxbWLSn6c2/X/RoMqtNa+pJ3VfrHaVmqFBt7 X-Received: by 2002:a50:cdca:0:b0:4a2:3d2e:6502 with SMTP id h10-20020a50cdca000000b004a23d2e6502mr10096319edj.4.1675415786257; Fri, 03 Feb 2023 01:16:26 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675415786; cv=none; d=google.com; s=arc-20160816; b=roX9P7ItOsP9oAiim5Qc0wR0n+yKPEyFCutDtFv0n22AZ/Q5LEvUgYtUx9+EX0tBSQ ty3wTxB5t8jBn2XhxI+VdFbZzSWg0/mFbJS7IHxDv2bXe8o+os7wRTGIp9VFod7k/lPo ljHB5uEu7wrQ2CojpWHaAV2Q9tMs1mfd4pZlRrZxR5toBGk9MQ04eIrbCT6Fg3owqsOs ot8fCikC5EudsIq8rvjTaDmwtGPkkwoz7OdU1xxDOMjqZVYFVb4h/jQMUfQtZ0tKhlOk CYrcLPqui4uQkDGEIlwgxbWTmwtdWQOUk/3fA2G7eu78M85tCQwg3URJuf8KBJWlw+Zz c1QQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:mime-version :user-agent:message-id:date:subject:mail-followup-to:to:dmarc-filter :delivered-to:dkim-signature:dkim-filter; bh=q5DTL6oFk3eQNHYqv3bda8N/GfVT/BTHan8bfapkePQ=; b=t41Gz615N9Vg3Cd7MgzCCwkM3VyyfrQXOFpTDfPtdLw5SwB1SZohcUWOX4k/vN3EXF DazFHK09JzLMQwLUZJM6smhEi7sXxau2+179tjPG4DL/qtzVd0cvx86kpIfAoVct8A66 z7VQVN6o9a/nGofgpbbkxZsSM09B++Zlu4DTxrAHQLJi9z5+h1o6l5zKH5q4JV2NsRn0 +R6wbZMJCrn3/WUdTDmZP6ngbRapLAriiHbmdO+9JkENe+KCnCJa9V7PxbUN8RJpLEt3 2Ot5g5iHCKStz1zx1xLmX+o9DZ8fhlIVMNg+Fiu26HlUCxbZW++Qsmph1uf5txQuyDg/ h8xw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=nG62iKbp; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id q9-20020a056402248900b00483bcf2ab07si2444619eda.325.2023.02.03.01.16.26 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Feb 2023 01:16:26 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=nG62iKbp; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2434D3858033 for ; Fri, 3 Feb 2023 09:16:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2434D3858033 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1675415784; bh=q5DTL6oFk3eQNHYqv3bda8N/GfVT/BTHan8bfapkePQ=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=nG62iKbp9+LA2QRfUFALwUEuLZRXjBPCqA1lKHnq3yK36QVFOghr2RoO3B84cQupn 5lTIyzU6g/PsjFhwiEQx4D2U6bHg8HznICRmsjbCedZWfjJHFxLrmqGVjgXGc0m7KV VCgTTI/CNSv7ewLgT9ZZtjZkiRWs2WKPIDjpviKc= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 667463858C52 for ; Fri, 3 Feb 2023 09:15:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 667463858C52 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1E0F2C14 for ; Fri, 3 Feb 2023 01:16:19 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.99.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8A7A63F8D6 for ; Fri, 3 Feb 2023 01:15:36 -0800 (PST) To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Subject: [PATCH] ifcvt: Fix regression in aarch64/fcsel_1.c Date: Fri, 03 Feb 2023 09:15:35 +0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-36.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, 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-Patchwork-Original-From: Richard Sandiford via Gcc-patches From: Richard Sandiford Reply-To: Richard Sandiford Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756800783531314505?= X-GMAIL-MSGID: =?utf-8?q?1756800783531314505?= aarch64/fcsel_1.c contains: double f_2 (double a, double b, double c, double d) { if (a > b) return c; else return d; } which started failing in the GCC 12 timeframe. When it passed, the RTL had the form: [A] (set (reg ret) (reg c)) (set (pc) (if_then_else (gt ...) (label_ref ret) (pc))) edge to ret, fallthru to else else: (set (reg ret) (reg d)) fallthru to ret ret: ...exit... i.e. a branch around. Now the RTL has form: [B] (set (reg ret) (reg d)) (set (pc) (if_then_else (gt ...) (label_ref then) (pc))) edge to then, fallthru to ret ret: ...exit... then: (set (reg ret) (reg c)) edge to ret i.e. a branch out. Both are valid, of course, and there's no easy way to predict which we'll get. But ifcvt canonicalises its representation on: if (cond) goto fallthru else goto non-fallthru That is, it canoncalises on the branch-around case for half-diamonds. It therefore wants to invert the comparison in [B] to get: if (...) goto ret else goto then But that isn't possible for strict FP gt, so the optimisation fails. Canonicalising on the branch-around case seems like the wrong choice for half diamonds. The natural way of expressing a conditional branch is for the label_ref to be the "then" destination and pc to be the "else" destination. And the natural choice of condition seems to be the one under which extra stuff *is* done, rather than the one under which extra stuff *isn't* done. But that decision goes back at least 20 years and it doesn't seem like a good idea to change it in stage 4. This patch instead allows the internal structure to store the condition in inverted form. For simplicity it handles only conditional moves, which is the one case that is needed to fix the known regression. (There are probably unknown regressions too, but still.) Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Richard gcc/ * ifcvt.h (noce_if_info::cond_inverted): New field. * ifcvt.cc (cond_move_convert_if_block): Swap the then and else values when cond_inverted is true. (noce_find_if_block): Allow the condition to be inverted when handling conditional moves. --- gcc/ifcvt.cc | 31 +++++++++++++++++++------------ gcc/ifcvt.h | 8 ++++++++ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc index 008796838f7..63ef42b3c34 100644 --- a/gcc/ifcvt.cc +++ b/gcc/ifcvt.cc @@ -4253,6 +4253,9 @@ cond_move_convert_if_block (struct noce_if_info *if_infop, e = dest; } + if (if_infop->cond_inverted) + std::swap (t, e); + target = noce_emit_cmove (if_infop, dest, code, cond_arg0, cond_arg1, t, e); if (!target) @@ -4405,7 +4408,6 @@ noce_find_if_block (basic_block test_bb, edge then_edge, edge else_edge, basic_block then_bb, else_bb, join_bb; bool then_else_reversed = false; rtx_insn *jump; - rtx cond; rtx_insn *cond_earliest; struct noce_if_info if_info; bool speed_p = optimize_bb_for_speed_p (test_bb); @@ -4481,25 +4483,28 @@ noce_find_if_block (basic_block test_bb, edge then_edge, edge else_edge, if (! onlyjump_p (jump)) return FALSE; - /* If this is not a standard conditional jump, we can't parse it. */ - cond = noce_get_condition (jump, &cond_earliest, then_else_reversed); - if (!cond) - return FALSE; - - /* We must be comparing objects whose modes imply the size. */ - if (GET_MODE (XEXP (cond, 0)) == BLKmode) - return FALSE; - /* Initialize an IF_INFO struct to pass around. */ memset (&if_info, 0, sizeof if_info); if_info.test_bb = test_bb; if_info.then_bb = then_bb; if_info.else_bb = else_bb; if_info.join_bb = join_bb; - if_info.cond = cond; + if_info.cond = noce_get_condition (jump, &cond_earliest, + then_else_reversed);; rtx_insn *rev_cond_earliest; if_info.rev_cond = noce_get_condition (jump, &rev_cond_earliest, !then_else_reversed); + if (!if_info.cond && !if_info.rev_cond) + return FALSE; + if (!if_info.cond) + { + std::swap (if_info.cond, if_info.rev_cond); + std::swap (cond_earliest, rev_cond_earliest); + if_info.cond_inverted = true; + } + /* We must be comparing objects whose modes imply the size. */ + if (GET_MODE (XEXP (if_info.cond, 0)) == BLKmode) + return FALSE; gcc_assert (if_info.rev_cond == NULL_RTX || rev_cond_earliest == cond_earliest); if_info.cond_earliest = cond_earliest; @@ -4518,7 +4523,9 @@ noce_find_if_block (basic_block test_bb, edge then_edge, edge else_edge, /* Do the real work. */ - if (noce_process_if_block (&if_info)) + /* ??? noce_process_if_block has not yet been updated to handle + inverted conditions. */ + if (!if_info.cond_inverted && noce_process_if_block (&if_info)) return TRUE; if (HAVE_conditional_move diff --git a/gcc/ifcvt.h b/gcc/ifcvt.h index c6ea244aae1..be1385aabe4 100644 --- a/gcc/ifcvt.h +++ b/gcc/ifcvt.h @@ -86,6 +86,14 @@ struct noce_if_info form as well. */ bool then_else_reversed; + /* True if THEN_BB is conditional on !COND rather than COND. + This is used if: + + - JUMP branches to THEN_BB on COND + - JUMP falls through to JOIN_BB on !COND + - COND cannot be reversed. */ + bool cond_inverted; + /* True if the contents of then_bb and else_bb are a simple single set instruction. */ bool then_simple;