From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by sourceware.org (Postfix) with ESMTP id DA670385E823 for ; Thu, 2 Apr 2020 08:28:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DA670385E823 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-237-xzrVbsjuO7acFuh5lb-mXw-1; Thu, 02 Apr 2020 04:28:42 -0400 X-MC-Unique: xzrVbsjuO7acFuh5lb-mXw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8E7E98017FD; Thu, 2 Apr 2020 08:28:41 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-113-52.ams2.redhat.com [10.36.113.52]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E18B3D768E; Thu, 2 Apr 2020 08:28:40 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id 0328SciU021467; Thu, 2 Apr 2020 10:28:38 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id 0328SYoB021466; Thu, 2 Apr 2020 10:28:34 +0200 Date: Thu, 2 Apr 2020 10:28:34 +0200 From: Jakub Jelinek To: "Kewen.Lin" Cc: GCC Patches , Bill Schmidt , Segher Boessenkool Subject: Re: [PATCH] Fix PR94401 by considering reverse overrun Message-ID: <20200402082834.GO2212@tucnak> Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.11.3 (2019-02-01) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Content-Disposition: inline X-Spam-Status: No, score=-20.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_2, GIT_PATCH_3, 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: Thu, 02 Apr 2020 08:28:48 -0000 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, gimpl= e_stmt_iterator *gsi, > =09=09=09 if (new_vtype !=3D NULL_TREE) > =09=09=09 ltype =3D half_vtype; > =09=09=09 } > +=09=09=09tree offset =3D dataref_offset > +=09=09=09=09=09? dataref_offset > +=09=09=09=09=09: 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 aroun= d the expression in this case. So: =09=09=09tree offset =3D (dataref_offset =09=09=09=09 ? dataref_offset =09=09=09=09 : build_int_cst (ref_type, 0)); or =09=09=09tree offset =09=09=09 =3D (dataref_offset =09=09=09 ? dataref_offset : build_int_cst (ref_type, 0)); > +=09=09=09if (ltype !=3D vectype > +=09=09=09 && memory_access_type =3D=3D VMAT_CONTIGUOUS_REVERSE) > +=09=09=09 offset =3D size_binop ( > +=09=09=09 PLUS_EXPR, > +=09=09=09 build_int_cst (ref_type, > +=09=09=09=09=09 DR_GROUP_GAP (first_stmt_info) > +=09=09=09=09=09 * tree_to_uhwi ( > +=09=09=09=09=09 TYPE_SIZE_UNIT (elem_type))), > +=09=09=09 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): =09=09=09 { =09=09=09 unsigned HOST_WIDE_INT gap =09=09=09 =3D DR_GROUP_GAP (first_stmt_info); =09=09=09 gap *=3D tree_to_uhwi (TYPE_SIZE_UNIT (elem_type)); =09=09=09 tree gapcst =3D build_int_cst (ref_type, gap); =09=09=09 offset =3D size_binop (PLUS_EXPR, offset, gapcst); =09=09=09 } =09Jakub