From patchwork Mon Aug 15 10:47:02 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 526 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6a10:38f:b0:2d5:3c95:9e21 with SMTP id 15csp1285299pxh; Mon, 15 Aug 2022 03:47:57 -0700 (PDT) X-Google-Smtp-Source: AA6agR5uWu8gZ/s9qIHO52sPJYEAjH+ovWOXcZZSflSJyWEw1vDktKWc5l/T/Mg9pQpTOZGjN6Y0 X-Received: by 2002:a05:6402:84e:b0:440:4bac:be5a with SMTP id b14-20020a056402084e00b004404bacbe5amr14080172edz.103.1660560477048; Mon, 15 Aug 2022 03:47:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1660560477; cv=none; d=google.com; s=arc-20160816; b=R6grbmYr5p1MAGbybH4070z/BSCDo0VN2ialKXLqMrDSyMqWKVx7Vzavw8lSpJnO0T ArfCsdB1mEOHp7YpP1W0GD56b+vE9ZHN2R4obS1Qvx4/9WyTztS0XzqZx4+5/DKfwaHS lgEuyEbhr1fTScxaOHOc/JF9YS3Fn5gZtrZPZ3i/oOaMqYaqbXXBmDMdhh77ePzOk9Xh /BRus0k6JQyF85NaTCcDNZ5Z5CcymarvCWxsw/sFcHnTDTCuvE6+ms8jOsXBlM11AcLt 13QN2k4h8WliDakClPRVL8Cq0uul1fKNqi+HkRMLUul6L8aSBVXAYYmdkco/7jFnDdxq Pe5g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:from:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence :content-disposition:mime-version:message-id:subject:to:date :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=xXA3mGcV7opuEE5wcEmy4zi3X4zn5j7uqwDF1PL5Y8c=; b=ttKFkyU2tHZWbBoCjWzdOZUykTpjiEWHuK5Z0fJ9UoS5eRBJLU5JKPD6n7Cxkbyf4y ACNOPB9ZHLPQgA8fpJEFDQ2KWl+0ZMxsK9P7PlkP8cu5xaK/be6mx/SmNfoAbq9BhI3Z 4rXdBgj3OeIO8GG0qJij7VjemIGoR2dku7tA3uJ7oXGh7H+LYGe2TiNEE2py23KncbQ8 dPQ3Wi1tvZDP81Vc/payW3t03/iVnkWrfMKzCv9Qkv/K0OiNEJ/s5ZeCKURqp4CT8mxP Zce8rCoKGfojwT+AQWZs4hGL/Cobz3N40dworED2HGfgb3u1AWYed462LgD2Zj+CQif2 t6xA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=pjX7iWYF; 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 (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id nc15-20020a1709071c0f00b007307c356ccesi7468790ejc.720.2022.08.15.03.47.56 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 Aug 2022 03:47:57 -0700 (PDT) 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=pjX7iWYF; 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 DD3863858429 for ; Mon, 15 Aug 2022 10:47:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DD3863858429 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1660560475; bh=xXA3mGcV7opuEE5wcEmy4zi3X4zn5j7uqwDF1PL5Y8c=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=pjX7iWYFqVtnyNaXqGYyrRGPTOnjIIFA0TfpRrzBRb/+h3I1IkLFRVewy/VmRwHhu O0/8CxC0VUWke8XpPGe/8uIGMxXn95gFK8ha8TRVkfyKpuRzB8HbQkxaUcDOeaniZa W+EjllXW4gpmE4EYCl1XgRDJrKTArcECKwa+j3CI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id AC7ED3858C74 for ; Mon, 15 Aug 2022 10:47:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AC7ED3858C74 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-536-U80ck-nZOYuKhwdwr5c4Eg-1; Mon, 15 Aug 2022 06:47:07 -0400 X-MC-Unique: U80ck-nZOYuKhwdwr5c4Eg-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 07C2C803301; Mon, 15 Aug 2022 10:47:07 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.41]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B5B4A40E80E3; Mon, 15 Aug 2022 10:47:06 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 27FAl3LW1240991 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 15 Aug 2022 12:47:04 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 27FAl2ZX1240990; Mon, 15 Aug 2022 12:47:02 +0200 Date: Mon, 15 Aug 2022 12:47:02 +0200 To: Richard Biener , Jeff Law Subject: [PATCH] ifcvt: Fix up noce_convert_multiple_sets [PR106590] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jakub Jelinek via Gcc-patches From: Jakub Jelinek Reply-To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org 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?1741223862586747881?= X-GMAIL-MSGID: =?utf-8?q?1741223862586747881?= Hi! The following testcase is miscompiled on x86_64-linux. The problem is in the noce_convert_multiple_sets optimization. We essentially have: if (g == 1) { g = 1; f = 23; } else { g = 2; f = 20; } and for each insn try to create a conditional move sequence. There is code to detect overlap with the regs used in the condition and the destinations, so we actually try to construct: tmp_g = g == 1 ? 1 : 2; f = g == 1 ? 23 : 20; g = tmp_g; which is fine. But, we actually try to create two different conditional move sequences in each case, seq1 with the whole (eq (reg/v:HI 82 [ g ]) (const_int 1 [0x1])) condition and seq2 with cc_cmp (eq (reg:CCZ 17 flags) (const_int 0 [0])) to rely on the earlier present comparison. In each case, we compare the rtx costs and choose the cheaper sequence (seq1 if both have the same cost). The problem is that with the skylake tuning, tmp_g = g == 1 ? 1 : 2; is actually expanded as tmp_g = (g == 1) + 1; in seq1 (which clobbers (reg 17 flags)) and as a cmov in seq2 (which doesn't). The tuning says both have the same cost, so we pick seq1. Next we check sequences for f = g == 1 ? 23 : 20; and here the seq2 cmov is cheaper, but it uses (reg 17 flags) which has been clobbered earlier. The following patch fixes that by detecting if we in the chosen sequence clobber some register mentioned in cc_cmp or rev_cc_cmp, and if yes, arranges for only seq1 (i.e. sequences that emit the comparison itself) to be used after that. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2022-08-15 Jakub Jelinek PR rtl-optimization/106590 * ifcvt.cc (check_for_cc_cmp_clobbers): New function. (noce_convert_multiple_sets_1): If SEQ sets or clobbers any regs mentioned in cc_cmp or rev_cc_cmp, don't consider seq2 for any further conditional moves. * gcc.dg/torture/pr106590.c: New test. Jakub --- gcc/ifcvt.cc.jj 2022-07-26 10:32:23.000000000 +0200 +++ gcc/ifcvt.cc 2022-08-12 16:31:45.348151269 +0200 @@ -3369,6 +3369,20 @@ noce_convert_multiple_sets (struct noce_ return TRUE; } +/* Helper function for noce_convert_multiple_sets_1. If store to + DEST can affect P[0] or P[1], clear P[0]. Called via note_stores. */ + +static void +check_for_cc_cmp_clobbers (rtx dest, const_rtx, void *p0) +{ + rtx *p = (rtx *) p0; + if (p[0] == NULL_RTX) + return; + if (reg_overlap_mentioned_p (dest, p[0]) + || (p[1] && reg_overlap_mentioned_p (dest, p[1]))) + p[0] = NULL_RTX; +} + /* This goes through all relevant insns of IF_INFO->then_bb and tries to create conditional moves. In case a simple move sufficis the insn should be listed in NEED_NO_CMOV. The rewired-src cases should be @@ -3519,7 +3533,7 @@ noce_convert_multiple_sets_1 (struct noc as min/max and emit an insn, accordingly. */ unsigned cost1 = 0, cost2 = 0; - rtx_insn *seq, *seq1, *seq2; + rtx_insn *seq, *seq1, *seq2 = NULL; rtx temp_dest = NULL_RTX, temp_dest1 = NULL_RTX, temp_dest2 = NULL_RTX; bool read_comparison = false; @@ -3531,9 +3545,10 @@ noce_convert_multiple_sets_1 (struct noc as well. This allows the backend to emit a cmov directly without creating an additional compare for each. If successful, costing is easier and this sequence is usually preferred. */ - seq2 = try_emit_cmove_seq (if_info, temp, cond, - new_val, old_val, need_cmov, - &cost2, &temp_dest2, cc_cmp, rev_cc_cmp); + if (cc_cmp) + seq2 = try_emit_cmove_seq (if_info, temp, cond, + new_val, old_val, need_cmov, + &cost2, &temp_dest2, cc_cmp, rev_cc_cmp); /* The backend might have created a sequence that uses the condition. Check this. */ @@ -3588,6 +3603,24 @@ noce_convert_multiple_sets_1 (struct noc return FALSE; } + if (cc_cmp) + { + /* Check if SEQ can clobber registers mentioned in + cc_cmp and/or rev_cc_cmp. If yes, we need to use + only seq1 from that point on. */ + rtx cc_cmp_pair[2] = { cc_cmp, rev_cc_cmp }; + for (walk = seq; walk; walk = NEXT_INSN (walk)) + { + note_stores (walk, check_for_cc_cmp_clobbers, cc_cmp_pair); + if (cc_cmp_pair[0] == NULL_RTX) + { + cc_cmp = NULL_RTX; + rev_cc_cmp = NULL_RTX; + break; + } + } + } + /* End the sub sequence and emit to the main sequence. */ emit_insn (seq); --- gcc/testsuite/gcc.dg/torture/pr106590.c.jj 2022-08-12 16:39:33.931965959 +0200 +++ gcc/testsuite/gcc.dg/torture/pr106590.c 2022-08-12 16:39:17.752179521 +0200 @@ -0,0 +1,75 @@ +/* PR rtl-optimization/106590 } */ +/* { dg-do run } */ +/* { dg-additional-options "-mtune=skylake" { target { i?86-*-* x86_64-*-* } } } */ + +typedef struct A { short a; } A; +typedef A *B; +typedef struct C { int c, d; } C; +typedef C *D; + +B +foo (void) +{ + static A r = { .a = 1 }; + return &r; +} + +D +bar (void) +{ + static C r = { .c = 1, .d = 23 }; + return &r; +} + +static inline int __attribute__((always_inline)) +baz (short a) +{ + int e = 1, f; + short g; + D h; + + switch (a) + { + case 1: + f = 23; + g = 1; + break; + case 2: + f = 20; + g = 2; + break; + } + + h = bar (); + + if (h->d != f || h->c != g) + __builtin_abort (); + return e; +} + +int +qux (void) +{ + B i = foo (); + int e = 1; + + switch (i->a) + { + case 1: + case 2: + e = baz (i->a); + break; + case 3: + e = 0; + break; + } + + return e; +} + +int +main () +{ + qux (); + return 0; +}