From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42494 invoked by alias); 26 Feb 2018 12:22:48 -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 42484 invoked by uid 89); 26 Feb 2018 12:22:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-lf0-f46.google.com Received: from mail-lf0-f46.google.com (HELO mail-lf0-f46.google.com) (209.85.215.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Feb 2018 12:22:46 +0000 Received: by mail-lf0-f46.google.com with SMTP id g72so2463839lfg.3 for ; Mon, 26 Feb 2018 04:22:45 -0800 (PST) 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; bh=ChU2kpV9diCGqqTGhxzAoaNm9VNRhel+LEc77VYCyts=; b=WOtJSz3nV65ZBo/Yine4BU6kqtVPxtj6VqXhjTVUAnDOX1G9zunZcqin7767E1YhQ4 bQVAGVHs0+6ClNp2YW6vnY7c6gabRRPyNUIE3AASc2uzyfZBoJrl7derS5Ch4HIr6UFq 4NLc7FprNuYZPMmkHXcw3r3GCor/XPi2Ng/y2YaadSHp4Fy6sLQHfJSMgcqUEYmLwAk5 WPE05ii3RSSvCCUn54SBg7zlH6p37D+4mUcumOkhGMenpQwoKtFt/dxWScs8FlGe91DZ iJGCSwihoamqGnctP5795lDQy8o+gep19gERDZO/XyoPxkEWPXFCimLsnvp7fBJRrCKb TbTA== X-Gm-Message-State: APf1xPAQP2bhO6NwgLhrcVCLVHx3LQ61GMiwf17OOxGno2Fsz8yO9ujL X6Wugx+PamOANdZhfycc8a/7KUYIKhYUfc7zsN9G0Q== X-Google-Smtp-Source: AH8x226WWAzZDs/BIcHEh95ZFD95xOhkQ0foeKsIucrJ0/KzZS90RQz9F2GHvXQsfQ85cjKq7sJEvxtDf8t4tWkSmaA= X-Received: by 10.25.222.207 with SMTP id i76mr6628178lfl.133.1519647763745; Mon, 26 Feb 2018 04:22:43 -0800 (PST) MIME-Version: 1.0 Received: by 10.46.95.143 with HTTP; Mon, 26 Feb 2018 04:22:43 -0800 (PST) In-Reply-To: <87606rsivf.fsf@linaro.org> References: <87606rsivf.fsf@linaro.org> From: Richard Biener Date: Mon, 26 Feb 2018 12:22:00 -0000 Message-ID: Subject: Re: Make fix for PR 83965 handle SLP reduction chains To: GCC Patches , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg01415.txt.bz2 On Tue, Feb 20, 2018 at 5:53 PM, Richard Sandiford wrote: > This patch prevents pattern-matching of fold-left SLP reduction chains, > which the previous patch for 83965 didn't handle properly. It only > stops the last statement in the group from being matched, but that's > enough to cause the group to be dissolved later. > > A better fix would be to put all the information about the reduction > on the the first statement in the reduction chain, so that every > statement in the group can tell what the group is doing. That doesn't > seem like stage 4 material though. > > As it stands, things seem to be a bit of a mess. In > vect_force_simple_reduction we attach the reduction type and > phi pointer to the last statement in a reduction chain: > > reduc_def_info = vinfo_for_stmt (def); > STMT_VINFO_REDUC_TYPE (reduc_def_info) = v_reduc_type; > STMT_VINFO_REDUC_DEF (reduc_def_info) = phi; > > and mark it as vect_reduction_type in vect_analyze_scalar_cycles_1: > > STMT_VINFO_DEF_TYPE (vinfo_for_stmt (reduc_stmt)) = > vect_reduction_def; > > This code in vectorizable_reduction gave the impression that > everything really is keyed off the last statement: > > /* In case of reduction chain we switch to the first stmt in the chain, but > we don't update STMT_INFO, since only the last stmt is marked as reduction > and has reduction properties. */ > if (GROUP_FIRST_ELEMENT (stmt_info) > && GROUP_FIRST_ELEMENT (stmt_info) != stmt) > { > stmt = GROUP_FIRST_ELEMENT (stmt_info); > first_p = false; > } > > But this code is dead these days. GROUP_FIRST_ELEMENT is only nonnull > for SLP reduction chains, since we dissolve the group if SLP fails. > And SLP only analyses the first statement in the group, not the last: > > stmt = SLP_TREE_SCALAR_STMTS (node)[0]; > stmt_vec_info stmt_info = vinfo_for_stmt (stmt); > [...] > bool res = vect_analyze_stmt (stmt, &dummy, node, node_instance); > > So from that point of view the DEF_TYPE, REDUC_TYPE and REDUC_DEF > are being attached to the wrong statement, since we only analyse > the first one. But it turns out that REDUC_TYPE and REDUC_DEF > don't matter for the first statement in the group, since that > takes the phi as an input, and when the phi is a direct input, > we use *its* REDUC_TYPE and REDUC_DEF instead of the statement's > own info. The DEF_TYPE problem is handled by: > > /* Mark the first element of the reduction chain as reduction to properly > transform the node. In the reduction analysis phase only the last > element of the chain is marked as reduction. */ > if (!STMT_VINFO_GROUPED_ACCESS (vinfo_for_stmt (stmt))) > STMT_VINFO_DEF_TYPE (vinfo_for_stmt (stmt)) = vect_reduction_def; > > in vect_analyze_slp_instance (cancelled by: > > STMT_VINFO_DEF_TYPE (vinfo_for_stmt (first_element)) > = vect_internal_def; > > in vect_analyze_slp on failure), with the operation being repeated > in vect_schedule_slp_instance (redundantly AFAICT): > > /* Mark the first element of the reduction chain as reduction to properly > transform the node. In the analysis phase only the last element of the > chain is marked as reduction. */ > if (GROUP_FIRST_ELEMENT (stmt_info) && !STMT_VINFO_GROUPED_ACCESS (stmt_info) > && GROUP_FIRST_ELEMENT (stmt_info) == stmt) > { > STMT_VINFO_DEF_TYPE (stmt_info) = vect_reduction_def; > STMT_VINFO_TYPE (stmt_info) = reduc_vec_info_type; > } > > Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu. > OK to install? Ok for stage1. Richard. > Richard > > > 2018-02-20 Richard Sandiford > > gcc/ > PR tree-optimization/83965 > * tree-vect-patterns.c (vect_reassociating_reduction_p): Assume > that grouped statements are part of a reduction chain. Return > true if the statement is not marked as a reduction itself but > is part of a group. > (vect_recog_dot_prod_pattern): Don't check whether the statement > is part of a group here. > (vect_recog_sad_pattern): Likewise. > (vect_recog_widen_sum_pattern): Likewise. > > gcc/testsuite/ > PR tree-optimization/83965 > * gcc.dg/vect/pr83965-2.c: New test. > > Index: gcc/tree-vect-patterns.c > =================================================================== > --- gcc/tree-vect-patterns.c 2018-02-20 09:40:41.843451227 +0000 > +++ gcc/tree-vect-patterns.c 2018-02-20 16:28:55.636762056 +0000 > @@ -222,13 +222,16 @@ vect_recog_temp_ssa_var (tree type, gimp > } > > /* Return true if STMT_VINFO describes a reduction for which reassociation > - is allowed. */ > + is allowed. If STMT_INFO is part of a group, assume that it's part of > + a reduction chain and optimistically assume that all statements > + except the last allow reassociation. */ > > static bool > vect_reassociating_reduction_p (stmt_vec_info stmt_vinfo) > { > return (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def > - && STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION); > + ? STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION > + : GROUP_FIRST_ELEMENT (stmt_vinfo) != NULL); > } > > /* Function vect_recog_dot_prod_pattern > @@ -350,8 +353,7 @@ vect_recog_dot_prod_pattern (vec { > gimple *def_stmt; > > - if (!vect_reassociating_reduction_p (stmt_vinfo) > - && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo)) > + if (!vect_reassociating_reduction_p (stmt_vinfo)) > return NULL; > oprnd0 = gimple_assign_rhs1 (last_stmt); > oprnd1 = gimple_assign_rhs2 (last_stmt); > @@ -571,8 +573,7 @@ vect_recog_sad_pattern (vec *s > { > gimple *def_stmt; > > - if (!vect_reassociating_reduction_p (stmt_vinfo) > - && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo)) > + if (!vect_reassociating_reduction_p (stmt_vinfo)) > return NULL; > plus_oprnd0 = gimple_assign_rhs1 (last_stmt); > plus_oprnd1 = gimple_assign_rhs2 (last_stmt); > @@ -1256,8 +1257,7 @@ vect_recog_widen_sum_pattern (vec if (gimple_assign_rhs_code (last_stmt) != PLUS_EXPR) > return NULL; > > - if (!vect_reassociating_reduction_p (stmt_vinfo) > - && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo)) > + if (!vect_reassociating_reduction_p (stmt_vinfo)) > return NULL; > > oprnd0 = gimple_assign_rhs1 (last_stmt); > Index: gcc/testsuite/gcc.dg/vect/pr83965-2.c > =================================================================== > --- /dev/null 2018-02-19 19:34:42.906488063 +0000 > +++ gcc/testsuite/gcc.dg/vect/pr83965-2.c 2018-02-20 16:28:55.635762095 +0000 > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-Ofast -ftrapv" } */ > + > +int c; > +unsigned char d; > +int e (unsigned char *f) > +{ > + int g; > + for (int a; a; a++) > + { > + for (int b = 0; b < 6; b++) > + g += __builtin_abs (f[b] - d); > + f += c; > + } > + return g; > +}