On Wed, 22 May 2024, Richard Sandiford wrote: > Richard Sandiford <richard.sandif...@arm.com> writes: > > Richard Biener <rguent...@suse.de> writes: > >> When change_vec_perm_layout runs into a permute combining two > >> nodes where one is invariant and one internal the partition of > >> one input can be -1 but the other might not be. The following > >> supports this case by simply ignoring inputs with input partiton -1. > >> > >> I'm not sure this is correct but it avoids ICEing when accessing > >> that partitions layout for gcc.target/i386/pr98928.c with the > >> change to avoid splitting store dataref groups during SLP discovery. > >> > >> Bootstrap and regtest running on x86_64-unknown-linux-gnu (ontop of > >> the SLP series). The change can't break anything that's already > >> broken but I'm not sure this does the right thing - the testcase > >> has an uniform constant. I'll try to come up with a better runtime > >> testcase tomorrow. Hints as to where to correctly fix such case > >> appreciated. > > > > Famous last words, but yeah, it looks correct to me. I think the > > routine in principle should have a free choice of which layout to > > choose for invariants (as long as it's consistent for all queries > > about the same node). So it should just be a question of whether > > keeping the original layout is more likely to give a valid > > permutation, or whether going with out_layout_i would be better. > > I don't have a strong intuition either way. > > BTW, I should have said that using a different layout from 0 > would require compensating code in the materialize function. > So this is definitely the simplest and most direct fix.
Yeah, I guess we can improve on that later. I'm going to push the change after lunch together with the other two fixes - the ARM CI discovered its share of testsuite fallout for the actual change I'm going to look at. Richard.
