Hi Richard, on 2020/7/22 下午2:38, Richard Biener wrote: > On Wed, Jul 22, 2020 at 3:26 AM Kewen.Lin <li...@linux.ibm.com> wrote: >> >> Hi Richard, >> >> on 2020/7/21 下午3:57, Richard Biener wrote: >>> On Tue, Jul 21, 2020 at 7:52 AM Kewen.Lin <li...@linux.ibm.com> wrote: >>>> >>>> Hi, >>>> >>>> This patch is to add the cost modeling for vector with length, >>>> it mainly follows what we generate for vector with length in >>>> functions vect_set_loop_controls_directly and vect_gen_len >>>> at the worst case. >>>> >>>> For Power, the length is expected to be in bits 0-7 (high bits), >>>> we have to model the cost of shifting bits. To allow other targets >>>> not suffer this, I used one target hook to describe this extra cost, >>>> I'm not sure if it's a correct way. >>>> >>>> Bootstrapped/regtested on powerpc64le-linux-gnu (P9) with explicit >>>> param vect-partial-vector-usage=1. >>>> >>>> Any comments/suggestions are highly appreciated! >>> >>> I don't like the introduction of an extra target hook for this. All >>> vectorizer cost modeling should ideally go through >>> init_cost/add_stmt_cost/finish_cost. If the extra costing is >>> not per stmt then either init_cost or finish_cost is appropriate. >>> Currently init_cost only gets a struct loop while we should >>> probably give it a vec_info * parameter so targets can >>> check LOOP_VINFO_USING_PARTIAL_VECTORS_P and friends. >>> >> >> Thanks! Nice, your suggested way looks better. I've removed the hook >> and taken care of it in finish_cost. The updated v2 is attached. >> >> Bootstrapped/regtested again on powerpc64le-linux-gnu (P9) with explicit >> param vect-partial-vector-usage=1. > > LGTM (with assuming the first larger hunk is mostly re-indenting > under LOOP_VINFO_USING_PARTIAL_VECTORS_P).
Thanks for the review! Yes, for the original LOOP_VINFO_FULLY_MASKED_P hunk, this patch moves the handling of gap peeling to be shared between mask and length, and re-indent the remaining (masking specific) into inner LOOP_VINFO_FULLY_MASKED_P. The length specific is put into the else hunk. It wouldn't change anything for masking, I'll run aarch64 regtesting to ensure it. :) BR, Kewen
