Hi Segher, Thanks for the comments!
on 2021/9/7 上午7:43, Segher Boessenkool wrote: > Hi! > > On Wed, Jul 28, 2021 at 10:59:50AM +0800, Kewen.Lin wrote: >>>> +/* As a visitor function for each statement cost entry handled in >>>> + function add_stmt_cost, gather some information and update its >>>> + relevant fields in target cost accordingly. */ >>> >>> I got lost trying to parse that.. (could be just me :-) >>> >>> Possibly instead something like >>> /* Helper function for add_stmt_cost ; gather information and update >>> the target_cost fields accordingly. */ >> >> OK, will update. I was thinking for each entry handled in function >> add_stmt_cost, this helper acts like a visitor, trying to visit each >> entry and take some actions if some conditions are satisifed. > > It (thankfully!) has nothing to do with the "visitor pattern", so some > other name might be better :-) > >>> Maybe clearer to read if you rearrange slightly and flatten it ? I >>> defer to others on that.. >>> >>> if ((kind == vec_to_scalar >>> || kind == vec_perm >>> || kind == vec_promote_demote >>> || kind == vec_construct >>> || kind == scalar_to_vec) >>> || (kind == vector_stmt && where == vect_body) >> >> This hunk is factored out from function rs6000_add_stmt_cost, maybe I >> can keep the original formatting? The formatting tool isn't so smart, >> and sometimes rearrange things to become unexpected (although it meets >> the basic rule, not so elegant), sigh. > > It has too many parens, making grouping where there is none, that is the > core issue. > > if (kind == vec_to_scalar > || kind == vec_perm > || kind == vec_promote_demote > || kind == vec_construct > || kind == scalar_to_vec > || (kind == vector_stmt && where == vect_body)) > > Good catch, I've updated it in V4. BR, Kewen
