On Wed, Jun 3, 2020 at 2:38 PM Richard Biener <richard.guent...@gmail.com> wrote: > > On June 3, 2020 8:23:14 PM GMT+02:00, Segher Boessenkool > <seg...@kernel.crashing.org> wrote: > >On Wed, Jun 03, 2020 at 07:23:47PM +0200, Richard Biener wrote: > >> >> mask = vec_cmp of the comparison > >> >> true_masked = true_op & mask; > >> >> false_masked = false_op & ~mask; > >> >> result = true_masked | false_masked; > >> >> > >> >> but I believe this would be dead code never triggered. > >> > > >> >But that would be the generic code as well? Is that not useful to > >have > >> >in any case? > >> > >> Sure. If you remove the vcond patterns from your port the vectorizer > >will do this transparently for you. So if you do not actually have a > >more clever way of representing this in the ISA there's no point of the > >vcond patterns. (though I think the vec_cmp ones didn't originally > >exist) > > > >So why can the expander not just do that whenever the patterns FAIL as > >well? > > It could but all the vectorizer costing assumed it goes the 'cheaper' way. So > this is kind of a sanity check. And what when vec_cmp expansion fails as > well? Resort to scalar soft FP support as ultimate fallback? That sounds very > wrong as a auto vectorization result... > > >> The point is the vectorizer relies on a optab query for querying > >backend support and power claims vcond support here. If you then FAIL > >you have lied. (not in your interpretation of the pattern docs but in > >the implementations since introduction of vcond named patterns) > > > >Almost all RTL patterns are allowed to FAIL, and that is a very good > >thing. If the vectoriser does not allow that, *it* is buggy. > > Your opinion. Please suggest a better way to query target vector capabilities. > > >> So if you're happy I'll document explicitly that vector named > >patterns may not FAIL. > > > >That will not work in general at all, no. Please document it for only > >those RTL patterns you need it for (and it is documented per pattern > >currently, anyway). > > Sure, will do. But as Richard said, the documented list is the other way > around. > At least that was my interpretation. > > All vectorizer queried optabs have this constraint BTW.
Looking at other primary targets, like x86, ARM, AArch64, I agree that other targets don't FAIL vector patterns in the same manner as scalar patterns. The design of Altivec support came with the work from RHES and no one at the time mentioned that FAILing those named patterns was incorrect. As Segher said, allowing named patterns to fail is fairly standard in GCC and the restriction for vector patterns seems to have appeared without warning or documentation. Maybe the vectorizer started to assume that (because of x86 behavior) but no one announced this. Again, I'm not saying that it is an incorrect design or policy, but that it seems to have been imposed retroactively. This needs to be documented. It needs to be applied uniformly throughout the common parts of the vectorizer and RTL generation. It needs to allow time for the Power port to adapt. And we need to test this thoroughly to catch unanticipated fallout. Thanks, David
