From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 82868 invoked by alias); 16 Aug 2017 10:47:25 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 82099 invoked by uid 89); 16 Aug 2017 10:47:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wr0-f177.google.com Received: from mail-wr0-f177.google.com (HELO mail-wr0-f177.google.com) (209.85.128.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 16 Aug 2017 10:47:22 +0000 Received: by mail-wr0-f177.google.com with SMTP id z91so2260845wrc.4 for ; Wed, 16 Aug 2017 03:47:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=a0ypH3AvEwmhMDLV1B6p6Km1E6LoOTkm51y+0OnoYeg=; b=FX3ikZ4QXK+UWF7+YK5IH1iXS2mipcf94Y9JiAuH9dpTyTEk7R0M2jHnqwNC4Rp6Rn 0NP5mJsghKUbsPaTApH4bbH9Rr7sK44KMgTX56EKIHkQi14VZtax28kSyMyuQa6lwyVD YdovxNQ89pJj3QUBoAJo2Bzfp1nKfYF/3cFUNg+PofVbClnupoM5WK7zR/A9Gayw93u3 1Xo7tbRZQv5iJflVZITO3jueAKHdP/+oXAT0wVxOHaoKlXwYWiCkdOUB4qXwDbQLwWyj AtY4IZmr/dDDi0qPB1zgBLY/fE1CSjsLXSckl7h2WiDpqIR4wEkduyWPNB/Ku2C3WeYF l6+A== X-Gm-Message-State: AHYfb5iSABF99GE0TxlpU8d90+flYnsdY0YAuHIp4ULkcLSnRwmJf9QL ruL4WyZUuIC1LsZVffZi60qK/MhY5A== X-Received: by 10.80.185.70 with SMTP id m64mr1618721ede.246.1502880440800; Wed, 16 Aug 2017 03:47:20 -0700 (PDT) MIME-Version: 1.0 Received: by 10.80.180.249 with HTTP; Wed, 16 Aug 2017 03:47:20 -0700 (PDT) In-Reply-To: References: <87fucsspl7.fsf@linaro.org> From: Richard Biener Date: Wed, 16 Aug 2017 10:56:00 -0000 Message-ID: Subject: Re: [PATCH PR81832]Skip copying loop header if inner loop is distributed To: "Bin.Cheng" Cc: "gcc-patches@gcc.gnu.org" , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00989.txt.bz2 On Wed, Aug 16, 2017 at 11:22 AM, Bin.Cheng wrote: > On Tue, Aug 15, 2017 at 6:33 PM, Richard Sandiford > wrote: >> Richard Biener writes: >>> On Tue, Aug 15, 2017 at 11:28 AM, Bin Cheng wrote: >>>> Hi, >>>> This patch fixes PR81832. Root cause for the ICE is: >>>> 1) Loop has distributed inner loop. >>>> 2) The guarding function call IFN_LOOP_DIST_CALL happens to be in loop's header. >>>> 3) IFN_LOOP_DIST_CALL (int loop's header) is duplicated by pass_ch_vect thus >>>> not eliminated. >>>> >>>> Given pass_ch_vect copies loop header to enable more vectorization, we should >>>> skip loop in this case because distributed inner loop means this loop can not >>>> be vectorized anyway. One point to mention is name inner_loop_distributed_p >>>> is a little misleading. The name indicates that each basic block is checked, >>>> but the patch only checks loop's header for simplicity/efficiency's purpose. >>>> Any comment? >>> >>> My comment would be to question pass_ch_vect placement -- what was the >>> reason to place it so late? >>> >>> I also see GRAPHITE runs inbetween loop distribution and vectorization -- >>> what prevents GRAPHITE from messing up things here? Or autopar? >>> >>> The patch itself shows should_duplicate_loop_header_p should >>> handle this IFN specially (somehow all IFNs are considered "inexpensive"). >>> >>> So can you please adjust should_duplicate_loop_header_p instead and/or >>> gimple_inexpensive_call_p? Since we have IFNs for stuff like EXP10 I'm not >>> sure if that default is so good. >> >> I think the default itself is OK: we only use IFNs for libm functions >> like exp10 if an underlying optab exists (unlike __builtin_exp10, which >> is useful as the non-errno setting version of exp10), so the target must >> have something that it thinks is worth open-coding. Also, we currently >> treat all MD built-ins as "simple" and thus "inexpensive", so the IFN >> handling is consistent with that. >> >> Maybe there are some IFNs that are worth special-casing as expensive, >> but IMO doing that to solve this problem would be a bit hacky. It seems >> like "inexpensive" should be more of a cost thing than a correctness thing. > Hi all, > This is updated patch. It only adds a single check on IFN_LOOP_DIST_ALIAS > in function should_duplicate_loop_header_p. As for gimple_inexpensive_call_p, > I think it's natural to consider functions like IFN_LOOP_VECTORIZED and > IFN_LOOP_DIST_ALIAS as expensive because they are only meant to indicate > temporary arrangement of optimizations and are never used in code generation. > I will send a standalone patch for that. Another thing is this patch > doesn't check > IFN_LOOP_VECTORIZED because it's impossible to have it with current order > of passes. > Bootstrap and test ongoing. Further comments? Ok. Thanks, Richard. > Thanks, > bin > 2017-08-15 Bin Cheng > > PR tree-optimization/81832 > * tree-ssa-loop-ch.c (should_duplicate_loop_header_p): Don't > copy loop header which has IFN_LOOP_DIST_ALIAS call. > > gcc/testsuite > 2017-08-15 Bin Cheng > > PR tree-optimization/81832 > * gcc.dg/tree-ssa/pr81832.c: New test. > >> >> Thanks, >> Richard >> >>> Thanks, >>> Richard. >>> >>>> Bootstrap and test on x86_64. >>>> >>>> Thanks, >>>> bin >>>> 2017-08-15 Bin Cheng >>>> >>>> PR tree-optimization/81832 >>>> * tree-ssa-loop-ch.c (inner_loop_distributed_p): New function. >>>> (pass_ch_vect::process_loop_p): Call above function. >>>> >>>> gcc/testsuite >>>> 2017-08-15 Bin Cheng >>>> >>>> PR tree-optimization/81832 >>>> * gcc.dg/tree-ssa/pr81832.c: New test.