"Yangfei (Felix)" <felix.y...@huawei.com> writes: > Hi, > >> -----Original Message----- >> From: Richard Sandiford [mailto:richard.sandif...@arm.com] >> Sent: Wednesday, July 1, 2020 9:03 PM >> To: Yangfei (Felix) <felix.y...@huawei.com> >> Cc: Richard Biener <rguent...@suse.de>; Richard Biener >> <richard.guent...@gmail.com>; gcc-patches@gcc.gnu.org >> Subject: Re: [PATCH PR95961] vect: ICE: in exact_div, at poly-int.h:2182 >> >> "Yangfei (Felix)" <felix.y...@huawei.com> writes: >> >> On June 30, 2020 4:23:03 PM GMT+02:00, Richard Sandiford >> >> <richard.sandif...@arm.com> wrote: >> >> >Richard Biener <rguent...@suse.de> writes: >> >> >> So it seems odd to somehow put in the number of vectors... so to >> >> >> me it would have made sense if it did >> >> >> >> >> >> possible_npeel_number = lower_bound (nscalars); >> >> >> >> >> >> or whateveris necessary to make the polys happy. Thus simply >> >> >> elide the vect_get_num_vectors call? But it's been very long >> >> >> since I've dived into the alignment peeling code... >> >> > >> >> >Ah, I see what you mean. So rather than: >> >> > >> >> > /* Save info about DR in the hash table. Also include peeling >> >> > amounts according to the explanation above. */ >> >> > for (j = 0; j < possible_npeel_number; j++) >> >> > { >> >> > vect_peeling_hash_insert (&peeling_htab, loop_vinfo, >> >> > dr_info, npeel_tmp); >> >> > npeel_tmp += target_align / dr_size; >> >> > } >> >> > >> >> >just have something like: >> >> > >> >> > while (known_le (npeel_tmp, nscalars)) >> >> > { >> >> > … >> >> > } >> >> > >> >> >? >> >> >> >> Yeah. >> > >> > Not sure if I understand correctly. I am supposing the following check in >> the original code is not necessary if we go like that. >> > >> > 1822 if (unlimited_cost_model (LOOP_VINFO_LOOP (loop_vinfo))) >> > >> > Is that correct? >> >> I think we still need it. I guess there are two choices: >> >> - make nscalars default to npeel_tmp before the “if” above. > > I think this will be simpler. How about the v2 patch? > Bootstrapped and tested on aarch64-linux-gnu & x86_64-linux-gnu. > > Thanks, > Felix > > From 69efce90fa746a2a86f8d673335b874c7df34d9f Mon Sep 17 00:00:00 2001 > From: Fei Yang <felix.y...@huawei.com> > Date: Thu, 2 Jul 2020 14:39:23 +0800 > Subject: [PATCH] vect: Fix an ICE in exact_div [PR95961] > > In the test case for PR95961, vectorization factor computed by > vect_determine_vectorization_factor is [8,8]. But this is updated to [1,1] > later by vect_update_vf_for_slp. When we call vect_get_num_vectors in > vect_enhance_data_refs_alignment, the number of scalars which is based on > the vectorization factor is not a multiple of the the number of elements in > the vector type. This leads to the ICE. This isn't a simple stream of > contiguous vector accesses. It's hard to predict from the available > information > how many vector accesses we'll actually need per iteration. As discussed, > here > we should use the number of scalars instead of the number of vectors as an > upper > bound for the loop saving info about DR in the hash table. > > 2020-07-02 Felix Yang <felix.y...@huawei.com> > > gcc/ > PR tree-optimization/95961 > * tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Use the > number of scalars instead of the number of vectors as an upper bound > for the loop saving info about DR in the hash table. Remove unused > local variables. > > gcc/testsuite/ > > PR tree-optimization/95961 > * gcc.target/aarch64/sve/pr95961.c: New test.
Thanks, pushed to master with a minor whitespace fix for… > + nscalars = (STMT_SLP_TYPE (stmt_info) > + ? vf * DR_GROUP_SIZE (stmt_info) : vf); …the indentation on this line. Hope you don't mind, but I also “reflowed” the commit message to make it fit within 72 chars. (The text itself is the same.) Richard
