| Message ID | 8760cknsr8.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix vectorizable_mask_load_store handling of invariant masks | expand |
On 09/15/2017 04:47 AM, Richard Sandiford wrote: > vectorizable_mask_load_store was not passing the required mask type to > vect_get_vec_def_for_operand. This doesn't matter for masks that are > defined in the loop, since their STMT_VINFO_VECTYPE will be what we need > anyway. But it's not possible to tell which mask type the caller needs > when looking at an invariant scalar boolean. As the comment above the > function says: > > In case OP is an invariant or constant, a new stmt that creates a vector def > needs to be introduced. VECTYPE may be used to specify a required type for > vector invariant. > > This fixes the attached testcase for SVE. > > Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. > OK to install? > > Richard > > > 2017-09-15 Richard Sandiford <richard.sandiford@linaro.org> > > gcc/ > * tree-vect-stmts.c (vectorizable_mask_load_store): Pass mask_vectype > to vect_get_vec_def_for_operand when getting the mask operand. > > gcc/testsuite/ > * gfortran.dg/vect/mask-store-1.f90: New test. OK. It does make me wonder though if we should just always be passing in the vectype rather than defaulting it to NULL to avoid similar issues at other call points. I haven't really looked at those call points to know if they're actually problematical or not -- it's just a general concern. Jeff
Index: gcc/tree-vect-stmts.c =================================================================== --- gcc/tree-vect-stmts.c 2017-09-15 11:36:58.331030784 +0100 +++ gcc/tree-vect-stmts.c 2017-09-15 11:39:17.850889641 +0100 @@ -2331,7 +2331,8 @@ vectorizable_mask_load_store (gimple *st { tree rhs = gimple_call_arg (stmt, 3); vec_rhs = vect_get_vec_def_for_operand (rhs, stmt); - vec_mask = vect_get_vec_def_for_operand (mask, stmt); + vec_mask = vect_get_vec_def_for_operand (mask, stmt, + mask_vectype); /* We should have catched mismatched types earlier. */ gcc_assert (useless_type_conversion_p (vectype, TREE_TYPE (vec_rhs))); @@ -2388,7 +2389,8 @@ vectorizable_mask_load_store (gimple *st if (i == 0) { - vec_mask = vect_get_vec_def_for_operand (mask, stmt); + vec_mask = vect_get_vec_def_for_operand (mask, stmt, + mask_vectype); dataref_ptr = vect_create_data_ref_ptr (stmt, vectype, NULL, NULL_TREE, &dummy, gsi, &ptr_incr, false, &inv_p); Index: gcc/testsuite/gfortran.dg/vect/mask-store-1.f90 =================================================================== --- /dev/null 2017-09-15 10:12:35.472207962 +0100 +++ gcc/testsuite/gfortran.dg/vect/mask-store-1.f90 2017-09-15 11:39:17.849889812 +0100 @@ -0,0 +1,11 @@ +subroutine foo(a, b, x, n) + real(kind=8) :: a(n), b(n), tmp + logical(kind=1) :: x + integer(kind=4) :: i, n + do i = 1, n + if (x) then + a(i) = b(i) + end if + b(i) = b(i) + 10 + end do +end subroutine