From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com [IPv6:2a00:1450:4864:20::52f]) by sourceware.org (Postfix) with ESMTPS id B31393858402 for ; Thu, 11 Nov 2021 10:58:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B31393858402 Received: by mail-ed1-x52f.google.com with SMTP id o8so22467942edc.3 for ; Thu, 11 Nov 2021 02:58:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mNQLYU5UWyfzvXhaOcwmM8v/cPcEnAU9ISl3ThHw9Mk=; b=axPDcFQSepPt63rPGGYpUVBdW2M17hjQkeJVgLTAGL2HYLFts3wwUU4xeul3AfdeC+ sHmi3d3UlOFPz+7MnHhezr9kbAB3tr8PvwHL0R9/08zjHNgE7fm7ZMDfZWiA5c8RYRnE exGR5XJKcoIW9215NLVgemGMOVrgEyGPXi0qj5jpkb8hJUyd/5ran2X1ISwERmBltfi9 rnKGhXEbNwx18TlMYPgDet5huprn+ilJ4v7znpudEdjszQdURD++AGpxr7Xl/ulJElNf /RyDDZevyfRVii2Kf/EbhrxkE3sGO+IpM9vJyL6O3KjdvjQ4+/iRN4eYtYbKSZwF9Y+6 lhkA== X-Gm-Message-State: AOAM533hbIwPiMv0Uunw+RFJX/10kFgo4bvk7zdopplyK/I48MaFB6NX GxwIbP9Ann8COgJF9qToR8aQbz7rAenyKiPPYKY= X-Google-Smtp-Source: ABdhPJzPLeGvZdawmkgvWDRI8f3mjh4bFBRxv+LMTmyxTSH4ScFLVT4Vw5yb5gLVrGwjmiwtSOUQsIFipU0/0sg5p+A= X-Received: by 2002:a17:906:c301:: with SMTP id s1mr8117280ejz.56.1636628321770; Thu, 11 Nov 2021 02:58:41 -0800 (PST) MIME-Version: 1.0 References: <20211110182003.686241-1-aldyh@redhat.com> <9f11bb40-7769-ff6b-72bf-f1eb48944760@gmail.com> In-Reply-To: From: Richard Biener Date: Thu, 11 Nov 2021 11:58:31 +0100 Message-ID: Subject: Re: [PATCH] Allow loop header copying when first iteration condition is known. To: Aldy Hernandez Cc: Jeff Law , GCC patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Thu, 11 Nov 2021 10:58:44 -0000 On Thu, Nov 11, 2021 at 11:33 AM Aldy Hernandez wrote: > > On Thu, Nov 11, 2021 at 8:30 AM Richard Biener > wrote: > > > > On Wed, Nov 10, 2021 at 9:42 PM Jeff Law wrote: > > > > > > > > > > > > On 11/10/2021 11:20 AM, Aldy Hernandez via Gcc-patches wrote: > > > > As discussed in the PR, the loop header copying pass avoids doing so > > > > when optimizing for size. However, sometimes we can determine the > > > > loop entry conditional statically for the first iteration of the loop. > > > > > > > > This patch uses the path solver to determine the outgoing edge > > > > out of preheader->header->xx. If so, it allows header copying. Doing > > > > this in the loop optimizer saves us from doing gymnastics in the > > > > threader which doesn't have the context to determine if a loop > > > > transformation is profitable. > > > > > > > > I am only returning true in entry_loop_condition_is_static for > > > > a true conditional. Technically a false conditional is also > > > > provably static, but allowing any boolean value causes a regression > > > > in gfortran.dg/vector_subscript_1.f90. > > > > > > > > I would have preferred not passing around the query object, but the > > > > layout of pass_ch and should_duplicate_loop_header_p make it a bit > > > > awkward to get it right without an outright refactor to the > > > > pass. > > > > > > > > Tested on x86-64 Linux. > > > > > > > > OK? > > > > > > > > gcc/ChangeLog: > > > > > > > > PR tree-optimization/102906 > > > > * tree-ssa-loop-ch.c (entry_loop_condition_is_static): New. > > > > (should_duplicate_loop_header_p): Call entry_loop_condition_is_static. > > > > (class ch_base): Add m_ranger and m_query. > > > > (ch_base::copy_headers): Pass m_query to > > > > entry_loop_condition_is_static. > > > > (pass_ch::execute): Allocate and deallocate m_ranger and > > > > m_query. > > > > (pass_ch_vect::execute): Same. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * gcc.dg/tree-ssa/pr102906.c: New test. > > > OK. It also makes a nice little example of how to use a Ranger within > > > an existing pass. > > > > Note if you just test for the condition to be true it will only catch 50% > > of the desired cases since we have no idea whether the 'true' edge > > is the edge existing the loop or the edge remaining in the loop. > > For loop header copying we like to resolve statically to the edge > > remaining in the loop, so you want > > Ahh, I figured there was some block shuffling needed. > > I was cautious not to touch much because of the > gfortran.dg/vector_subscript_1.f90 regression, but now I see that the > test fails for all optimization levels except -Os. With this fix we > properly fail for all levels. I assume this is expected ;-). > > > > > extract_true_false_edges_from_block (gimple_bb (last), &true_e, &false_e); > > > > /* If neither edge is the exit edge this is not a case we'd like to > > special-case. */ > > if (!loop_exit_edge_p (l, true_e) && !loop_exit_edge_p (l, false_e)) > > return false; > > > > tree desired_static_value; > > if (loop_exit_edge_p (l, true_e)) > > desired_static_value = boolean_false_node; > > else > > desired_static_value = boolean_true_node; > > > > and test for desired_static_value. > > Thanks for the code! > > OK pending tests? OK, thanks! Richard.