On Thu, May 20, 2021 at 12:09 PM Kewen.Lin <li...@linux.ibm.com> wrote: > > on 2021/5/20 下午5:30, Christophe Lyon wrote: > > On Thu, 20 May 2021 at 10:52, Kewen.Lin via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > >> > >> on 2021/5/19 下午6:01, Richard Biener wrote: > >>> On Wed, May 19, 2021 at 11:47 AM Kewen.Lin <li...@linux.ibm.com> wrote: > >>>> > >>>> Hi Richi, > >>>> > >>>> on 2021/5/19 下午4:15, Richard Biener wrote: > >>>>> On Wed, May 19, 2021 at 8:20 AM Kewen.Lin <li...@linux.ibm.com> wrote: > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> This patch is to replace the current hardcoded weight factor 50 > >>>>>> for those statements in an inner loop relative to the loop being > >>>>>> vectorized with a specific parameter vect-inner-loop-weight-factor. > >>>>>> > >>>>>> The motivation behind this change is: if targets want to have one > >>>>>> unique function to gather some information in each add_stmt_cost > >>>>>> call, no matter that it's put before or after the cost tweaking > >>>>>> part for inner loop, it may have the need to adjust (expand or > >>>>>> shrink) the gathered data as the factor. Now the factor is > >>>>>> hardcoded, it's not easily maintained. Since it's possible that > >>>>>> targets have their own decisions on this costing like the others, > >>>>>> I used parameter instead of one unique macro here. > >>>>>> > >>>>>> Testing is ongoing, is it ok for trunk if everything goes well? > >>>>> > >>>>> Certainly an improvement. I suppose we might want to put > >>>>> the factor into vinfo->inner_loop_cost_factor. That way > >>>>> we could adjust it easily in common code in the vectorizer > >>>>> when we for example have (non-guessed) profile data. > >>>>> > >>>>> "weight_factor" is kind-of double-speak and I'm missing 'cost' ... > >>>>> so, bike-shedding to vect_inner_loop_cost_factor? > >>>>> > >>>>> Just suggestions - as said, the patch is an improvement already. > >>>>> > >>>> > >>>> Thanks for your nice suggestions! I've updated the patch accordingly > >>>> and attached it. Does it look better to you? > >>> > >>> Minor nit: > >>> > >>> +@item vect-inner-loop-cost-factor > >>> +The factor which loop vectorizer uses to over weight those statements in > >>> +an inner loop relative to the loop being vectorized. > >>> + > >>> > >>> the default value should be documented here, not.. > >>> > >>> +-param=vect-inner-loop-cost-factor= > >>> +Common Joined UInteger Var(param_vect_inner_loop_cost_factor) > >>> Init(50) IntegerRange(1, 999999) Param Optimization > >>> +Indicates the factor which loop vectorizer uses to over weight those > >>> statements in an inner loop relative to the loop being vectorized. > >>> The default value is 50. > >>> + > >>> > >>> here (based on statistical analysis of existing cases). Also the > >>> params.opt docs > >>> should be the "brief" one - but for simplicity simply make both docs > >>> identical > >>> (apart from the default value doc). I suggest > >>> > >>> "The factor which the loop vectorizer applies to the cost of statements > >>> in an inner loop relative to the loop being vectorized." > >>> > >> > >> Thanks for catching this and the suggestion! > >> > >> Bootstrapped/regtested on powerpc64le-linux-gnu P9, x86_64-redhat-linux > >> and aarch64-linux-gnu. > >> > > > > This breaks the build for arm targets: > > /tmp/158661_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/arm/arm.c: > > In function 'unsigned int arm_add_stmt_cost(vec_info*, void*, int, > > vect_cost_for_stmt, _stmt_v > > ec_info*, tree, int, vect_cost_model_location)': > > /tmp/158661_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/arm/arm.c:12230:4: > > error: 'loop_vec_info' was not declared in this scope > > loop_vec_info loop_vinfo = dyn_cast<loop_vec_info> (vinfo); > > ^ > > /tmp/158661_3.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/arm/arm.c:12230:18: > > error: expected ';' before 'loop_vinfo' > > loop_vec_info loop_vinfo = dyn_cast<loop_vec_info> (vinfo); > > > > Can you fix it? > > > > Oops! Deeply sorry for that and thanks for the testing! > > I just found that unlike the other targets arm.c doesn't include > "tree-vectorizer.h". The issue should be fixed with the below patch: > > gcc/ChangeLog: > > * config/arm/arm.c: Include head files tree-vectorizer.h and > cfgloop.h. > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index caf4e56b9fe..6ed34fbf627 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -69,6 +69,8 @@ > #include "gimplify.h" > #include "gimple.h" > #include "selftest.h" > +#include "cfgloop.h" > +#include "tree-vectorizer.h" > > /* This file should be included last. */ > #include "target-def.h" > > > Is it counted as a obvious patch?
Please check if you can build a cc1 cross to arm, then yes. > BR, > Kewen
