From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x641.google.com (mail-ej1-x641.google.com [IPv6:2a00:1450:4864:20::641]) by sourceware.org (Postfix) with ESMTPS id D724F3870846 for ; Fri, 31 Jul 2020 13:01:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D724F3870846 Received: by mail-ej1-x641.google.com with SMTP id a26so5535230ejc.2 for ; Fri, 31 Jul 2020 06:01:38 -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:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=LB/dAu1z8gdmaHpZtVXioKkeSCMVx8FHMqrvsEL0yaA=; b=nG9iTIjIttfQ0LzYcTmZ34Ly6evoKnqY7NF88oUAKKjrNueb9QYlGCsv6Tv1YXCyuv qjld7o0DaPNQNfrj9v35g9FBnBNArGPuazLEmdNR/Fdutz/ESmx68V1rN/VfOqMQzRsh xniLawRYuATEMr+ULCj+JsDuqGuJWj2jHeuh2zZCA4OIPtWjJ8J3DY1ctHx56h2w2NOP KPEbPBXxak4ThMDbP9yIOOQSNUnNKozkjJWRLZrcUg0/8XprafNmV/lL3FvwcZM2zAre S1PD8qAxMgEJNUu41hFVa3xEOIBOAmJbN7qWlC36PsajPYe4B7patz4bNDwCGb4CcQaz 38AQ== X-Gm-Message-State: AOAM532x+z7IPmhXVmnizVRO/Vsk+SiMMSllSMn4+Od4SP7IS0Bxsecj BlYqolbs3VcOE1XhLvvP5IEzL6Q3jtrZkizFfjQ= X-Google-Smtp-Source: ABdhPJzc03EWS4zV58dV23NRFxmkH8U/voLPOfv+GkH1HA3C69R6GBXaAJWzP1E9qNEtdYBtcjkJI1uQw/Bc4PffK8A= X-Received: by 2002:a17:906:384a:: with SMTP id w10mr4115292ejc.235.1596200497879; Fri, 31 Jul 2020 06:01:37 -0700 (PDT) MIME-Version: 1.0 References: <419f1fad-05be-115c-1a53-cb710ae7b2dc@linux.ibm.com> <1aeabdc7-0cf4-055b-a3ec-74c283053cf5@linux.ibm.com> <7d90d154-6a76-c8f5-75f6-1e1057dfcc0f@linux.ibm.com> <4c7c935b-8eaa-1fd7-4357-72fd3239e710@linux.ibm.com> In-Reply-To: <4c7c935b-8eaa-1fd7-4357-72fd3239e710@linux.ibm.com> From: Richard Biener Date: Fri, 31 Jul 2020 15:01:26 +0200 Message-ID: Subject: Re: [PATCH v4] vect/rs6000: Support vector with length cost modeling To: "Kewen.Lin" Cc: Richard Sandiford , GCC Patches , Bill Schmidt , Segher Boessenkool Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.0 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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Fri, 31 Jul 2020 13:01:40 -0000 On Fri, Jul 31, 2020 at 2:37 PM Kewen.Lin wrote: > > Hi Richards, > > on 2020/7/31 =E4=B8=8B=E5=8D=887:20, Richard Biener wrote: > > On Fri, Jul 31, 2020 at 1:03 PM Richard Sandiford > > wrote: > >> > >> "Kewen.Lin" writes: > >>>>> + bool niters_known_p =3D LOOP_VINFO_NITERS_KNOWN_P (loop_vinf= o); > >>>>> + bool need_iterate_p > >>>>> + =3D (!LOOP_VINFO_EPILOGUE_P (loop_vinfo) > >>>>> + && !vect_known_niters_smaller_than_vf (loop_vinfo)); > >>>>> + > >>>>> + /* Init min/max, shift and minus cost relative to single > >>>>> + scalar_stmt. For now we only use length-based partial vectors = on > >>>>> + Power, target specific cost tweaking may be needed for other > >>>>> + ports in future. */ > >>>>> + unsigned int min_max_cost =3D 2; > >>>>> + unsigned int shift_cost =3D 1, minus_cost =3D 1; > >>>> > >>>> Please instead add a scalar_min_max to vect_cost_for_stmt, and use > >>>> scalar_stmt for shift and minus. There shouldn't be any Power thing= s > >>>> hard-coded into target-independent code. > >>>> > >>> > >>> Agree! It's not good to leave them there. I thought to wait and see > >>> if other targets which support vector with length can reuse this, or > >>> move it to target specific codes then if not sharable. But anyway > >>> it looks not good, let's fix it. > > > > In other generic places like this we simply use three generic scalar_st= mt > > costs. At least I don't see how min_max should be different from it > > when shift can be the same as minus. Note this is also how we treat > > Yeah, normally they (min/max/minus/shift) are taken as scalar_stmt, excep= ting > for fine-grain tuning like i386 port, they will use the same cost. On Po= wer9, > to implement min/max it takes double cycles of the normal scalar operatio= ns > like add/shift, I was trying to model it more fine-grained since we proba= bly > generate a few min/max here, if the loop body cost is small, I was worrie= d > the decision isn't good enough. But yeah, in other generic places, the s= mall > loop could also suffer this similar off, they are the same essentially. > > > vectorization of MAX_EXPR - scalar cost is one scalar_stmt and > > vector cost is one vector_stmt. As you say below the add_stmt_cost > > hook can adjust based on the actual GIMPLE stmt -- if one is > > available (which indeed it isn't here). > > > > I'm somewhat lacking context here as well - we actually GIMPLE > > code-generate the min/max/shift/minus and only the eventual > > AND is defered to the target, right? > > > > Yes, min/max/shift/minus are all GIMPLE code, targets like Power > will have its target specific cost for shift which moves length > to high bits 0:7. > > One typical case is as below: > > [local count: 105119324]: > _26 =3D n_11(D) * 4; > _37 =3D MAX_EXPR <_26, 16>; > _38 =3D _37 + 18446744073709551600; > _40 =3D MIN_EXPR <_26, 16>; > > [local count: 630715945]: > # ivtmp_35 =3D PHI <0(3), ivtmp_36(4)> > # loop_len_30 =3D PHI <_40(3), _44(4)> > _19 =3D &MEM[base: a_12(D), index: ivtmp_35, offset: 0B]; > vect_24 =3D .LEN_LOAD (_19, 4B, loop_len_30); > vect__3.7_23 =3D VIEW_CONVERT_EXPR(vect_24); > _1 =3D &MEM[base: b_13(D), index: ivtmp_35, offset: 0B]; > vect_17 =3D .LEN_LOAD (_1, 4B, loop_len_30); > vect__5.10_9 =3D VIEW_CONVERT_EXPR(vect_17); > vect__7.11_8 =3D vect__5.10_9 + vect__3.7_23; > vect_28 =3D VIEW_CONVERT_EXPR(vect__7.11_8); > _2 =3D &MEM[base: c_14(D), index: ivtmp_35, offset: 0B]; > .LEN_STORE (_2, 4B, loop_len_30, vect_28); > _42 =3D MIN_EXPR ; > _43 =3D _38 - _42; > _44 =3D MIN_EXPR <_43, 16>; > ivtmp_36 =3D ivtmp_35 + 16; > if (ivtmp_35 < _38) > goto ; [83.33%] > else > goto ; [16.67%] > > > >>> I had some concerns on vect_cost_for_stmt way, since it seems to allo= w > >>> more computations similar to min/max to be added like this, in long > >>> term it probably leads to the situtation like: scalar_min_max, > >>> scalar_div_expr, scalar_mul_expr ... an enum (cost types) bloat, it > >>> seems not good to maintain. > >> > >> I guess doing that doesn't seem so bad to me :-) I think it's been > >> a recurring problem that the current classification isn't fine-grained > >> enough for some cases. > > > > But we eventually want to get rid of this classification enum in favor > > of the add_stmt_cost hook ... > > > > Nice, sounds like each target has to handle it fine-grain. :) > IIUC, the current modeling doesn't consider the instruction dependency an= d > execution resource etc. like scheduling, even if all costs are fine-grain= ed > enough, the decision could be sub-optimal. That's what the finish_cost hook is for - the target can collect all stmts during add_stmt and then apply adjustments at the end (IIRC power does already for shift operation resource constraints). > >>> I noticed that i386 port ix86_add_stmt_cost will check stmt_info->stm= t, > >>> whether is assignment and the subcode of the expression, it provides = the > >>> chance to check the statement more fine-grain, not just as normal > >>> scalar_stmt/vector_stmt. > >>> > >>> For the case here, we don't have the stmt_info, but we know the type > >>> of computation(expression), how about to extend the hook add_stmt_cos= t > >>> with one extra tree_code type argument, by default it can be some > >>> unmeaningful code, for some needs like here, we specify the tree_code > >>> as the code of computation, like {MIN,MAX}_EXPR, then target specific > >>> add_stmt_cost can check this tree_code and adjust the cost accordingl= y. > >> > >> If we do that, I guess we should =E2=80=9Cpromote=E2=80=9D code_helper= out of > >> gimple-match.h and use that instead, so that we can handle > >> internal and built-in functions too. > >> > >> Would like to hear Richard's opinion on the best way forward here. > > > > I'd say defer this to a later patch and for now simply cost one scalar > > stmt for MIN/MAX. I agree that if we add a tree_code we want a > > code_helper instead. Note that I want to eventually have a > > full SLP tree for the final code generation where all info should be > > there (including SLP nodes for those min/max ops) and which the > > target could traverse. But I'm not sure if I can make enough progress > > on that SLP-only thing for GCC 11 even... > > > > OK, I'm fine to take MIN/MAX as scalar_stmt here. Thank both of you! > This new SLP framework looks very promising and powerful. :-) As do all things that are still on paper^Win my head only ;) Richard. > > BR, > Kewen