On Fri, 25 Oct 2019, Andre Vieira (lists) wrote: > > > On 22/10/2019 14:56, Richard Biener wrote: > > On Tue, 22 Oct 2019, Andre Vieira (lists) wrote: > > > >> Hi Richi, > >> > >> See inline responses to your comments. > >> > >> On 11/10/2019 13:57, Richard Biener wrote: > >>> On Thu, 10 Oct 2019, Andre Vieira (lists) wrote: > >>> > >>>> Hi, > >>>> > >> > >>> > >>> + > >>> + /* Keep track of vector sizes we know we can vectorize the epilogue > >>> with. */ > >>> + vector_sizes epilogue_vsizes; > >>> }; > >>> > >>> please don't enlarge struct loop, instead track this somewhere > >>> in the vectorizer (in loop_vinfo? I see you already have > >>> epilogue_vinfos there - so the loop_vinfo simply lacks > >>> convenient access to the vector_size?) I don't see any > >>> use that could be trivially adjusted to look at a loop_vinfo > >>> member instead. > >> > >> Done. > >>> > >>> For the vect_update_inits_of_drs this means that we'd possibly > >>> do less CSE. Not sure if really an issue. > >> > >> CSE of what exactly? You are afraid we are repeating a calculation here we > >> have done elsewhere before? > > > > All uses of those inits now possibly get the expression instead of > > just the SSA name we inserted code for once. But as said, we'll see. > > > > This code changed after some comments from Richard Sandiford. > > > + /* We are done vectorizing the main loop, so now we update the > > epilogues > > + stmt_vec_info's. At the same time we set the gimple UID of each > > + statement in the epilogue, as these are used to look them up in the > > + epilogues loop_vec_info later. We also keep track of what > > + stmt_vec_info's have PATTERN_DEF_SEQ's and RELATED_STMT's that might > > + need updating and we construct a mapping between variables defined > > in > > + the main loop and their corresponding names in epilogue. */ > > + for (unsigned i = 0; i < epilogue->num_nodes; ++i) > > > > so for the following code I wonder if you can make use of the > > fact that loop copying also copies UIDs, so you should be able > > to match stmts via their UIDs and get at the other loop infos > > stmt_info by the copy loop stmt UID. > > > > I wonder why you need no modification for the SLP tree? > > > I checked with Tamar and the SLP tree works with the position of operands and > not SSA_NAMES. So we should be fine.
There's now SLP_TREE_SCALAR_OPS but only for invariants so I guess we should indeed be fine here. Everything else is already stmt_infos which you patch with the new underlying stmts. Richard.
