Message ID | 20120517.034445.1589480557721203986.davem@davemloft.net |
---|---|
State | New |
Headers | show |
On Thu, 17 May 2012, David Miller wrote: > > Richard, I was looking into a testsuite failure on the 4.7 branch > on sparc and I think your fix for 52584 would fix it too. > > The problem eminates in gcc.c-torture/execute/vector-shift2.c with -O0 > > The 4 integer vector shifts get lowered to 2 integer vector shifts, > since that is what the VIS3 shifts are capable of. > > However, type_for_widest_vector_mode does not take the signedness into > consideration. So: > > signed_vector >> unsigned_vector > > gets lowered into: > > unsigned_vector >> unsigned_vector > > since type_for_widest_vector_mode unconditionally always passes > '1' for "unsignedp", and thus the testcase aborts since we use > a logical shift instead of an arithmetic one. > > I came up with the crude fix below, but then I noticed in mainline > the change you made in order to resolve 52584. Backporting that change is fine with me if you can take care to do the bootstrap & testing on the 4.7 branch. Thanks, Richard. > I'd like to see this fixed on the 4.7 branch since the problem > generates incorrect code, rather than merely miss an optimization > opportunity. > > diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c > index be54710..aa7b774 100644 > --- a/gcc/tree-vect-generic.c > +++ b/gcc/tree-vect-generic.c > @@ -476,7 +476,8 @@ expand_vector_operation (gimple_stmt_iterator *gsi, tree type, tree compute_type > SATP is true for saturating fixed-point types. */ > > static tree > -type_for_widest_vector_mode (enum machine_mode inner_mode, optab op, int satp) > +type_for_widest_vector_mode (enum machine_mode inner_mode, optab op, int satp, > + int unsignedp) > { > enum machine_mode best_mode = VOIDmode, mode; > int best_nunits = 0; > @@ -508,7 +509,7 @@ type_for_widest_vector_mode (enum machine_mode inner_mode, optab op, int satp) > if (ALL_FIXED_POINT_MODE_P (best_mode)) > return lang_hooks.types.type_for_mode (best_mode, satp); > > - return lang_hooks.types.type_for_mode (best_mode, 1); > + return lang_hooks.types.type_for_mode (best_mode, unsignedp); > } > } > > @@ -858,7 +859,8 @@ expand_vector_operations_1 (gimple_stmt_iterator *gsi) > { > tree vector_compute_type > = type_for_widest_vector_mode (TYPE_MODE (TREE_TYPE (type)), op, > - TYPE_SATURATING (TREE_TYPE (type))); > + TYPE_SATURATING (TREE_TYPE (type)), > + TYPE_UNSIGNED (TREE_TYPE (type))); > if (vector_compute_type != NULL_TREE > && (TYPE_VECTOR_SUBPARTS (vector_compute_type) > < TYPE_VECTOR_SUBPARTS (compute_type)) > >
From: Richard Guenther <rguenther@suse.de> Date: Fri, 18 May 2012 11:48:55 +0200 (CEST) > On Thu, 17 May 2012, David Miller wrote: > >> >> Richard, I was looking into a testsuite failure on the 4.7 branch >> on sparc and I think your fix for 52584 would fix it too. >> >> The problem eminates in gcc.c-torture/execute/vector-shift2.c with -O0 >> >> The 4 integer vector shifts get lowered to 2 integer vector shifts, >> since that is what the VIS3 shifts are capable of. >> >> However, type_for_widest_vector_mode does not take the signedness into >> consideration. So: >> >> signed_vector >> unsigned_vector >> >> gets lowered into: >> >> unsigned_vector >> unsigned_vector >> >> since type_for_widest_vector_mode unconditionally always passes >> '1' for "unsignedp", and thus the testcase aborts since we use >> a logical shift instead of an arithmetic one. >> >> I came up with the crude fix below, but then I noticed in mainline >> the change you made in order to resolve 52584. > > Backporting that change is fine with me if you can take care to > do the bootstrap & testing on the 4.7 branch. Great, I'll start doing that right now.
diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c index be54710..aa7b774 100644 --- a/gcc/tree-vect-generic.c +++ b/gcc/tree-vect-generic.c @@ -476,7 +476,8 @@ expand_vector_operation (gimple_stmt_iterator *gsi, tree type, tree compute_type SATP is true for saturating fixed-point types. */ static tree -type_for_widest_vector_mode (enum machine_mode inner_mode, optab op, int satp) +type_for_widest_vector_mode (enum machine_mode inner_mode, optab op, int satp, + int unsignedp) { enum machine_mode best_mode = VOIDmode, mode; int best_nunits = 0; @@ -508,7 +509,7 @@ type_for_widest_vector_mode (enum machine_mode inner_mode, optab op, int satp) if (ALL_FIXED_POINT_MODE_P (best_mode)) return lang_hooks.types.type_for_mode (best_mode, satp); - return lang_hooks.types.type_for_mode (best_mode, 1); + return lang_hooks.types.type_for_mode (best_mode, unsignedp); } } @@ -858,7 +859,8 @@ expand_vector_operations_1 (gimple_stmt_iterator *gsi) { tree vector_compute_type = type_for_widest_vector_mode (TYPE_MODE (TREE_TYPE (type)), op, - TYPE_SATURATING (TREE_TYPE (type))); + TYPE_SATURATING (TREE_TYPE (type)), + TYPE_UNSIGNED (TREE_TYPE (type))); if (vector_compute_type != NULL_TREE && (TYPE_VECTOR_SUBPARTS (vector_compute_type) < TYPE_VECTOR_SUBPARTS (compute_type))