------- Comment #6 from rguenther at suse dot de 2008-09-14 12:53 ------- Subject: Re: [4.4 Regression] Revision 140257 causes vectorizer tests failures
On Sun, 14 Sep 2008, irar at il dot ibm dot com wrote: > ------- Comment #5 from irar at il dot ibm dot com 2008-09-14 12:05 ------- > (In reply to comment #4) > > For the 32bit case the problem is really the choice of the vector > > type for the store (where is this decided on?). As the type of > > b is int it should have chosen vector int instead of vector long. > > Now I am completely confused. The decision to base the type of the store on > the > rhs was made to fix pr 37385 > (http://gcc.gnu.org/ml/gcc-patches/2008-09/msg00674.html). Without it, there > is ICE in set_mem_alias_set... Sorry, I probably didn't look too closely when reviewing this patch. > Vector pointers are created in vect_create_data_ref_ptr. When called from > vectorizable_store, it gets the type of vectorized rhs and creates a vector > pointer of that type (this was the patch for pr 37385). The sanity checking should forgive any wrong decision we make there, still basing it on the lhs looks more correct. I will have a look myself. The important thing is to make both the creation and the check consistent of course. I see vect_create_data_ref_ptr is getting the type to use passed in case of stores and this is TREE_TYPE (vec_oprnd) - is vec_oprnd the lhs or the rhs? It looks like it is the rhs which may explain the ICE for PR37385, we should use the vector type according to the lhs here (maybe passing NULL to vect_create_data_ref_ptr will do this - dependend on how STMT_VINFO_VECTYPE is set for stores). > > Note that it is perfectly valid (on 32bit) to assign a vector long > > to a vector int. So with a change like > > > > > The vectorized version (if the alias check is removed) is: > > > ... > > > vector long int * ivtmp.120; > > > > vector int * ivtmp.120; > > > > > vector long int vect_var_.113; > > > ... > > > > > > vect_var_.111_20 = *ivtmp.110_18; > > > ivtmp.110_21 = ivtmp.110_18 + 16; > > > vect_var_.112_22 = *ivtmp.110_21; > > > vect_var_.113_23 = __builtin_ia32_vec_pack_sfix (vect_var_.111_20, > > > vect_var_.112_22); > > > *ivtmp.120_26 = vect_var_.113_23; > > > > > > The alias check is for the store, checking *ivtmp.120_26 and b. > > > > the alias check would be fine. > > This part I do understand ;) Good ;) > > > > So, > > > > /* The type of the vector store is determined by the rhs. */ > > vectype = get_vectype_for_scalar_type (TREE_TYPE (op)); > > > > the type should be determined by the lhs > > not if we base the vectype on the scalar type of the rhs... > > > (after all we try to check > > if the new vector lhs aliases the old scalar lhs). But of course > > this means the vector lhs type should be chosen to actually match > > the scalar type of the lhs. > > > > if (!useless_type_conversion_p (TREE_TYPE (op), TREE_TYPE > > (scalar_dest))) > > { > > if (vect_print_dump_info (REPORT_DETAILS)) > > fprintf (vect_dump, "operands of different types"); > > return false; > > } > > > > This test should then be adjusted to check > > > > if (!useless_type_conversion_p (TREE_TYPE (TREE_TYPE > > (vectype)), TREE_TYPE (op))) > > > > first, it was the wrong way around, second we should check if the > > conversion from the rhs (op) to the element type of the lhs > > vector type (vectype) is useless. > > Do you mean > if (!useless_type_conversion_p (TREE_TYPE (scalar_dest), TREE_TYPE (op)))? > If vectype is built based on TREE_TYPE (scalar_dest) it should be the same, > no? It should be the same - ideally we'd check the conversion on both vector types (the lhs and the rhs). > > Now the interesting part is of course where we select the type > > for the induction variable for the store (I can't find this > > atm). > > Isn't it ivtmp.120_26 itself? (The updating of vector pointers is done in > vect_create_data_ref_ptr as well). Yes, looks like vect_create_data_ref_ptr decides if it is not passed the type (which it is for stores). I can try to have a look here, but I'm not exactly familiar with the code (thanks for the pointers ;)), so I'd appreciate if you try to figure out what it takes to fix vect_create_data_ref_ptr or its caller. Thanks, Richard. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37491
