Hi! On Wed, Jun 03, 2020 at 04:46:12PM +0200, Richard Biener wrote: > On Wed, Jun 3, 2020 at 4:17 PM David Edelsohn <dje....@gmail.com> wrote: > > On Wed, Jun 3, 2020 at 9:41 AM Richard Sandiford > > <richard.sandif...@arm.com> wrote: > > > Well, it seems unfortunate to have to do that. > > > > > > I think Martin's powerpc patch is the correct one.
It is papering over the issues a little -- the same assumption is made at lower levels as well, so all *that* needs to be changed as well (not "fixed", it is not a bug, we have a change in the vcond* interface here; oh and that should be documented as well). > > How about (3) help to remove reliance on this incorrect behavior from > > the PowerPC port? It is not a reliance on incorrect behaviour. This is a change. Which pretty much everyone seems to want, so fine, but that takes time. > > I didn't formally check, but if this is 16 years old, then it's from > > the original RHES Altivec work. It is, exactly. > > I don't believe that anyone fundamentally is objecting to "fixing this > > correctly". I don't know the entire history of this discussion, but > > my objection is to a fix that breaks a long-time assumption of the > > PowerPC port and leaves it as an exercise to the PowerPC maintainers > > to fix it. *Exactly*. This is changing an ancient interface, claiming "it always was that way" (which very obviously isn't true), and leaving the rs6000 people to deal with all the fallout. Again. > I _think_ there's nothing to fix besides removing the FAIL. All the lower levels need to get asserts as well. We need a week or so to put the whole thing through the wringer. The documentation needs to be changed by whoever changes the vcond* semantics. All other ports should be checked, too. > And I would > have no idea how to "fix" the powerpc port here since a) we lack a testcase > that actually FAILs, b) I'm not familiar with the ISA. So we did (3) by > replacing the FAILs with gcc_unreachable () and bootstrap/regtest this > without any regression which I think "proves" the failure modes do not > actually exist. Heh, assuming the testsuite is comprehensive? Heh. (Bootstrap doesn't mean much for vector code). > So I'm not sure how we can help. You'll have to document the "vcond* is not allowed to FAIL" change. We'll deal with the rest. But testing needs a week or so. (That is an extremely short timescale already). > A vcond can usually be emulated by vec_cmp plus masking. That would be the generic way to implement this of course, but apparently such code doesn't yet exist? If there is a generic implementation it should be trivial to deal with FAILs. > So if > we ever get a testcase that runs into the gcc_unreachable () I'll promise > to fix it up using this strategy in the vcond expander. But without a > testcase and powerpc ISA knowledge it's really hard. Or do you want > us to stick the vec_cmp expansion fallback in place of the FAILs? > I'm sure the powerpc maintainers are better suited to do that even though > I'll probably manage with some cut&paste. To recap: vcond is > equal to > > 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? Segher
