From patchwork Mon Oct 18 10:17:16 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 68163 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 2A36BB70EE for ; Mon, 18 Oct 2010 21:17:30 +1100 (EST) Received: (qmail 32351 invoked by alias); 18 Oct 2010 10:17:28 -0000 Received: (qmail 32339 invoked by uid 22791); 18 Oct 2010 10:17:26 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, TW_TM, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor.suse.de (HELO mx1.suse.de) (195.135.220.2) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 18 Oct 2010 10:17:20 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.suse.de (Postfix) with ESMTP id 1C9485362F; Mon, 18 Oct 2010 12:17:17 +0200 (CEST) Date: Mon, 18 Oct 2010 12:17:16 +0200 (CEST) From: Richard Guenther To: Ira Rosen Cc: gcc-patches@gcc.gnu.org Subject: Re: [patch] Fix PR tree-optimization/46049 and tree-optimization/46052 In-Reply-To: Message-ID: References: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Sun, 17 Oct 2010, Ira Rosen wrote: > > Hi, > > With this patch we choose operands' type as the type of the vector created > for them in case when the operands are invariant variables. > In these PRs we have two different operations, so it doesn't seem > appropriate to choose the type according to operation. > > OK for trunk after bootstrap and testing complete on x86_64-suse-linux? Well, I now looked at the problem myself. The function seems to be used to create vectors for operands which then get used to create vectorized statements to produce their LHS (thus, much similar to vect_get_vec_def_for_operand). It thus does not make much sense to look at STMT_VINFO_VECTYPE at all (that's the vector type of the LHS of the stmt with the operands). It should simply use get_vectype_for_scalar_type always (or get_same_sized_vectype with STMT_VINFO_VECTYPE as the vector type determining the size). So either gcc_assert (vector_type); nunits = TYPE_VECTOR_SUBPARTS (vector_type); both variants pass the new testcases and the C vect testsuite for me. Unless we start being less conservative with constraining vector sizes with AVX there is probably no functional difference, but for consistency I'd prefer the second variant. Thanks, Richard. > Thanks, > Ira > > ChangeLog: > > PR tree-optimization/46049 > PR tree-optimization/46052 > * tree-vect-slp.c (vect_get_constant_vectors): Use operands' > type in case of invariant variables. > > testsuite/ChangeLog: > > PR tree-optimization/46049 > PR tree-optimization/46052 > * gcc.dg/vect/pr46052.c: New test. > * gcc.dg/vect/pr46049.c: New test. > > > Index: testsuite/gcc.dg/vect/pr46052.c > =================================================================== > --- testsuite/gcc.dg/vect/pr46052.c (revision 0) > +++ testsuite/gcc.dg/vect/pr46052.c (revision 0) > @@ -0,0 +1,33 @@ > +/* { dg-do compile } */ > + > +int i; > +int a[2]; > + > +static inline char bar (void) > +{ > + return i ? i : 1; > +} > + > +void foo (int n) > +{ > + while (n--) > + { > + a[0] ^= bar (); > + a[1] ^= bar (); > + } > +} > + > +static inline char bar1 (void) > +{ > +} > + > +void foo1 (int n) > +{ > + while (n--) > + { > + a[0] ^= bar1 (); > + a[1] ^= bar1 (); > + } > +} > + > +/* { dg-final { cleanup-tree-dump "vect" } } */ > Index: testsuite/gcc.dg/vect/pr46049.c > =================================================================== > --- testsuite/gcc.dg/vect/pr46049.c (revision 0) > +++ testsuite/gcc.dg/vect/pr46049.c (revision 0) > @@ -0,0 +1,21 @@ > +/* { dg-do compile } */ > + > +typedef __INT16_TYPE__ int16_t; > +typedef __INT32_TYPE__ int32_t; > + > +static inline int32_t bar (int16_t x, int16_t y) > +{ > + return x * y; > +} > + > +void foo (int16_t i, int16_t *p, int16_t x) > +{ > + while (i--) > + { > + *p = bar (*p, x) >> 15; > + p++; > + *p = bar (*p, x) >> 15; > + p++; > + } > +} > +/* { dg-final { cleanup-tree-dump "vect" } } */ > Index: tree-vect-slp.c > =================================================================== > --- tree-vect-slp.c (revision 165574) > +++ tree-vect-slp.c (working copy) > @@ -1902,8 +1902,8 @@ vect_get_constant_vectors (slp_tree slp_ > /* For POINTER_PLUS_EXPR we use the type of the constant/invariant > itself. > If OP is the first operand of POINTER_PLUS_EXPR, its type is the type > of > the statement, so it's OK to use OP's type for both first and second > - operands. */ > - if (code == POINTER_PLUS_EXPR) > + operands. We also use the type of OP if it's an invariant variable. > */ > + if (code == POINTER_PLUS_EXPR || !constant_p) > vector_type = get_vectype_for_scalar_type (TREE_TYPE (op)); > else > vector_type = STMT_VINFO_VECTYPE (stmt_vinfo); > > Index: gcc/tree-vect-slp.c =================================================================== --- gcc/tree-vect-slp.c (revision 165610) +++ gcc/tree-vect-slp.c (working copy) @@ -1899,14 +1899,7 @@ vect_get_constant_vectors (slp_tree slp_ else constant_p = false; - /* For POINTER_PLUS_EXPR we use the type of the constant/invariant itself. - If OP is the first operand of POINTER_PLUS_EXPR, its type is the type of - the statement, so it's OK to use OP's type for both first and second - operands. */ - if (code == POINTER_PLUS_EXPR) - vector_type = get_vectype_for_scalar_type (TREE_TYPE (op)); - else - vector_type = STMT_VINFO_VECTYPE (stmt_vinfo); + vector_type = get_vectype_for_scalar_type (TREE_TYPE (op)); gcc_assert (vector_type); nunits = TYPE_VECTOR_SUBPARTS (vector_type); or, to probably make AVX happier with float -> int conversion Index: gcc/tree-vect-slp.c =================================================================== --- gcc/tree-vect-slp.c (revision 165610) +++ gcc/tree-vect-slp.c (working copy) @@ -1899,14 +1899,8 @@ vect_get_constant_vectors (slp_tree slp_ else constant_p = false; - /* For POINTER_PLUS_EXPR we use the type of the constant/invariant itself. - If OP is the first operand of POINTER_PLUS_EXPR, its type is the type of - the statement, so it's OK to use OP's type for both first and second - operands. */ - if (code == POINTER_PLUS_EXPR) - vector_type = get_vectype_for_scalar_type (TREE_TYPE (op)); - else - vector_type = STMT_VINFO_VECTYPE (stmt_vinfo); + vector_type = get_same_sized_vectype (TREE_TYPE (op), + STMT_VINFO_VECTYPE (stmt_vinfo));