Hi, on 2020/4/2 下午4:28, Jakub Jelinek wrote: > Hi! > > On Thu, Apr 02, 2020 at 03:15:42PM +0800, Kewen.Lin via Gcc-patches wrote: > > Just formatting nits, not commenting on what the actual patch does. > >> --- a/gcc/tree-vect-stmts.c >> +++ b/gcc/tree-vect-stmts.c >> @@ -9590,11 +9590,20 @@ vectorizable_load (stmt_vec_info stmt_info, >> gimple_stmt_iterator *gsi, >> if (new_vtype != NULL_TREE) >> ltype = half_vtype; >> } >> + tree offset = dataref_offset >> + ? dataref_offset >> + : build_int_cst (ref_type, 0); > > The above is misformatted. The ? and : shouldn't be indented further than > the dataref_offset, but usually e.g. for the sake of emacs we add ()s around > the expression in this case. So: > tree offset = (dataref_offset > ? dataref_offset > : build_int_cst (ref_type, 0)); > or > tree offset > = (dataref_offset > ? dataref_offset : build_int_cst (ref_type, 0)); >
Thanks Jakub! I'll follow this by add () for ternary expression. With manual added "()", clang-format can get below: tree offset = (dataref_offset ? dataref_offset : build_int_cst (ref_type, 0)); contrib/check_GNU_style.sh didn't complain this, I'm not sure whether it's possible to add this kind of convention into contrib/clang-format. >> + if (ltype != vectype >> + && memory_access_type == VMAT_CONTIGUOUS_REVERSE) >> + offset = size_binop ( >> + PLUS_EXPR, >> + build_int_cst (ref_type, >> + DR_GROUP_GAP (first_stmt_info) >> + * tree_to_uhwi ( >> + TYPE_SIZE_UNIT (elem_type))), >> + offset); > > Again, no reason to indent * by 2 columns from DR_GROUP_GAP. But also all > the (s at the end of line and randomly indented arguments look ugly. > I'd recommend temporaries, e.g. like (perhaps with different names of > temporaries, so that they don't shadow anything): > > { > unsigned HOST_WIDE_INT gap > = DR_GROUP_GAP (first_stmt_info); > gap *= tree_to_uhwi (TYPE_SIZE_UNIT (elem_type)); > tree gapcst = build_int_cst (ref_type, gap); > offset = size_binop (PLUS_EXPR, offset, gapcst); > } > Good suggestion, will update it. BR, Kewen