From patchwork Fri Aug 25 12:37:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 136914 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a7d1:0:b0:3f2:4152:657d with SMTP id p17csp1767019vqm; Fri, 25 Aug 2023 05:38:21 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEafdQEOQ66NXUtDZ93Ov9ssrOQjXXjuQZ1m1xKXXAme80Q1j6v1suQwDBoAIqtmSdfkCYj X-Received: by 2002:a2e:9ad5:0:b0:2bc:b6b0:1c4d with SMTP id p21-20020a2e9ad5000000b002bcb6b01c4dmr12907411ljj.10.1692967101241; Fri, 25 Aug 2023 05:38:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1692967101; cv=none; d=google.com; s=arc-20160816; b=huHkW1Z1JK9WDxm0g+oHSjGINRjjY9/fESZi7Rvyvx9AdbjG4UgQjvoRrHFPCgCZUT a112fDQD9IyHjBysiTUcVmGT64d6BJZ7O8Py6Z0jUQinhdb7Y9RmtCWztF2zyHYcmBVm Qb0XId8BGPbqZbMhYDDepZkvRrG5dCg0jvATlanSH8knieRS+RA6lCY/RqEFcuCIRfKm m9bB7y/qUeLlKnOEZVRJ0xxNQtxI1WMF9K1mItlnxnv0Ps5Vf/4L6ieIIJ8unYwbpEud oudmBTBj9EVyj/zxKD3m6GMTHnb+BFPRioGgbH/cvvEYagZij8EOYDgPDugTedU0U0ID c50w== 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:message-id :mime-version:subject:to:date:dmarc-filter:delivered-to :dkim-signature:dkim-filter; bh=1GvNNp4znbSnPaJ55PSdTiPMRNBM/CQn1SBK1ck/jeI=; fh=hPrbWPhweUx4V0GV9uXJqbyAzg2ABmTz7kczrAQqMmM=; b=jmHUT/4bnOT5Yjw9NIeqXa1Dip++DSsYhAJvZC1GkIs//aRz1T3Qpt7OrwNrik6OHt Cekp6XOu4sFPzoVMdZktioaXVQxlqvfEYGXTV/asskS+08Nx+38neEkr3fckR/G0U1n7 k5wNyHJamQRVvjygDeLu+MgYxZkyoQe9hI4KCW6XHl2tvSqko2xdW2vq8Ko0UrJgKWKw 3MgZkrH4SWkrkqOu0Im5DDCkzGC9SNA0E6ksB1Ip0PYxlPiSGf6AUWBDCDQ3e4zUB2tA /0PbG+gloN2eI32P7QRV/NV2QSNGi1hiHEIN3lneLUU47RvxV7wXCYdlJP7eXQ7xTCvO MzyQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=jXUbcqdJ; 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 (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id h1-20020a1709062dc100b0099b94cd668bsi906512eji.298.2023.08.25.05.38.20 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Aug 2023 05:38:21 -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=jXUbcqdJ; 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 E7B5B38582A4 for ; Fri, 25 Aug 2023 12:38:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E7B5B38582A4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1692967093; bh=1GvNNp4znbSnPaJ55PSdTiPMRNBM/CQn1SBK1ck/jeI=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=jXUbcqdJiEb8hDytKSbVLVVGax9hqPzJb0ZLs6GNvcjOMvVzD8OpwLac7RqgcdEgP J7ql++Wgo+yz/3hRERgDhtrIeFjT0A/m+BjC7/gHfJp850QGT+q436XHOdL4HNUBht FGI9gxz5PBs/tDPAkqXcrbwtKqqD0kfeKuACzU58= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 3F7E73858C53 for ; Fri, 25 Aug 2023 12:37:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3F7E73858C53 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 68AB22200D for ; Fri, 25 Aug 2023 12:37:28 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 531381340A for ; Fri, 25 Aug 2023 12:37:28 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id bt/0Eoig6GTbXgAAMHmgww (envelope-from ) for ; Fri, 25 Aug 2023 12:37:28 +0000 Date: Fri, 25 Aug 2023 14:37:27 +0200 (CEST) To: gcc-patches@gcc.gnu.org Subject: [PATCH] tree-optimization/111137 - dependence checking for SLP MIME-Version: 1.0 Message-Id: <20230825123728.531381340A@imap2.suse-dmz.suse.de> X-Spam-Status: No, score=-11.5 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 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 Biener via Gcc-patches From: Richard Biener Reply-To: Richard Biener Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1775204671045871901 X-GMAIL-MSGID: 1775204671045871901 The following fixes a mistake with SLP dependence checking. When checking whether we can hoist loads to the first load place we special-case stores of the same instance considering them sunk to the last store place. But we fail to consider that stores from other SLP instances are sunk in a similar way. This leads us to miss the dependence between (A) and (B) in b[0][1] = 0; (A) ... _6 = b[_5 /* 0 */][0]; (B') _7 = _6 ^ 1; b[_5 /* 0 */][0] = _7; b[0][2] = 0; (A') _10 = b[_5 /* 0 */][1]; (B) _11 = _10 ^ 1; b[_5 /* 0 */][1] = _11; where the zeroing stores are sunk to (A') and the loads hoisted to (B'). The following fixes this, treating grouped stores from other instances similar to stores from our own instance. The difference is - and this is more conservative than necessary - that we don't know which stores of a group are in which SLP instance (though I believe either all of the grouped stores will be in a single SLP instance or in none at the moment), so we don't know which stores are sunk where. We simply assume they are all sunk to the last store we run into. Likewise we do not take into account that an SLP instance might be cancelled (or a grouped store not actually belong to any instance). Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/111137 * tree-vect-data-refs.cc (vect_slp_analyze_load_dependences): Properly handle grouped stores from other SLP instances. * gcc.dg/torture/pr111137.c: New testcase. --- gcc/testsuite/gcc.dg/torture/pr111137.c | 30 ++++++++++++ gcc/tree-vect-data-refs.cc | 64 ++++++++++++++++++------- 2 files changed, 77 insertions(+), 17 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr111137.c diff --git a/gcc/testsuite/gcc.dg/torture/pr111137.c b/gcc/testsuite/gcc.dg/torture/pr111137.c new file mode 100644 index 00000000000..77560487926 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr111137.c @@ -0,0 +1,30 @@ +/* { dg-do run } */ + +int b[3][8]; +short d; +volatile int t = 1; + +void __attribute__((noipa)) +foo() +{ + int g = t; + for (int e = 1; e >= 0; e--) + { + d = 1; + for (; d >= 0; d--) + { + b[0][d * 2 + 1] = 0; + b[g - 1 + d][0] ^= 1; + b[0][d * 2 + 2] = 0; + b[g - 1 + d][1] ^= 1; + } + } +} + +int +main() +{ + foo (); + if (b[0][1] != 1) + __builtin_abort(); +} diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc index 0295e256a44..40ab568fe35 100644 --- a/gcc/tree-vect-data-refs.cc +++ b/gcc/tree-vect-data-refs.cc @@ -750,6 +750,7 @@ vect_slp_analyze_load_dependences (vec_info *vinfo, slp_tree node, data_reference *dr_a = STMT_VINFO_DATA_REF (access_info); ao_ref ref; bool ref_initialized_p = false; + hash_set grp_visited; for (gimple_stmt_iterator gsi = gsi_for_stmt (access_info->stmt); gsi_stmt (gsi) != first_access_info->stmt; gsi_prev (&gsi)) { @@ -782,26 +783,55 @@ vect_slp_analyze_load_dependences (vec_info *vinfo, slp_tree node, continue; } - /* We are hoisting a load - this means we can use TBAA for - disambiguation. */ - if (!ref_initialized_p) - ao_ref_init (&ref, DR_REF (dr_a)); - if (stmt_may_clobber_ref_p_1 (stmt, &ref, true)) + auto check_hoist = [&] (stmt_vec_info stmt_info) -> bool { - /* If we couldn't record a (single) data reference for this - stmt we have to give up now. */ - data_reference *dr_b = STMT_VINFO_DATA_REF (stmt_info); - if (!dr_b) - return false; - ddr_p ddr = initialize_data_dependence_relation (dr_a, - dr_b, vNULL); - bool dependent - = vect_slp_analyze_data_ref_dependence (vinfo, ddr); - free_dependence_relation (ddr); - if (dependent) + /* We are hoisting a load - this means we can use TBAA for + disambiguation. */ + if (!ref_initialized_p) + ao_ref_init (&ref, DR_REF (dr_a)); + if (stmt_may_clobber_ref_p_1 (stmt_info->stmt, &ref, true)) + { + /* If we couldn't record a (single) data reference for this + stmt we have to give up now. */ + data_reference *dr_b = STMT_VINFO_DATA_REF (stmt_info); + if (!dr_b) + return false; + ddr_p ddr = initialize_data_dependence_relation (dr_a, + dr_b, vNULL); + bool dependent + = vect_slp_analyze_data_ref_dependence (vinfo, ddr); + free_dependence_relation (ddr); + if (dependent) + return false; + } + /* No dependence. */ + return true; + }; + if (STMT_VINFO_GROUPED_ACCESS (stmt_info)) + { + /* When we run into a store group we have to honor + that earlier stores might be moved here. We don't + know exactly which and where to since we lack a + back-mapping from DR to SLP node, so assume all + earlier stores are sunk here. It's enough to + consider the last stmt of a group for this. + ??? Both this and the fact that we disregard that + the conflicting instance might be removed later + is overly conservative. */ + if (!grp_visited.add (DR_GROUP_FIRST_ELEMENT (stmt_info))) + for (auto store_info = DR_GROUP_FIRST_ELEMENT (stmt_info); + store_info != NULL; + store_info = DR_GROUP_NEXT_ELEMENT (store_info)) + if ((store_info == stmt_info + || get_later_stmt (store_info, stmt_info) == stmt_info) + && !check_hoist (store_info)) + return false; + } + else + { + if (!check_hoist (stmt_info)) return false; } - /* No dependence. */ } } return true;