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: > > > > Richard Biener <richard.guent...@gmail.com> writes: > > > On Tue, Jun 2, 2020 at 5:00 PM Martin Liška <mli...@suse.cz> wrote: > > >> > > >> On 6/2/20 1:09 PM, Richard Biener wrote: > > >> > So please be constructive. Like, provide a testcase that ICEs > > >> > with the FAILs replaced by gcc_unreachable (). Martin, may I suggest > > >> > to do this replacement and bootstrap/test? I think it would be nice > > >> > to have testsuite coverage for the FAILs, and maybe we have that > > >> > already. > > >> > > >> Hello. > > >> > > >> There's the suggested patch that survives bootstrap on ppc64le-linux-gnu > > >> and passes test-suite. > > > > > > OK, so can you please re-post the version of the VEC_COND_EXPR > > > patch that uses a regular IFN (without the static non-FAIL checking) > > > in a new thread? If there's no OK from rs6000 maintainers to remove > > > the FAILs then we'll go ahead with that version, unless Richard objects > > > here. > > > > Well, it seems unfortunate to have to do that. > > > > I think Martin's powerpc patch is the correct one. But assuming that > > the powerpc maintainers still object, I guess the options are: > > > > - Find enough global reviewers who are prepared to approve that patch, > > to override the powerpc maintainers. > > Luckily GCC Development does not operate this way. > > How about (3) help to remove reliance on this incorrect behavior from > the PowerPC port? > > I didn't formally check, but if this is 16 years old, then it's from > the original RHES Altivec work. > > 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.
I _think_ there's nothing to fix besides removing the FAIL. 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. So I'm not sure how we can help. There's the option to remove the vcond_* patterns entirely which makes us fall back to the vec_cmp_* ones which do never FAIL on powerpc. But I suspect this might regress code generation. It's suspicious that vec_cmp_* never FAIL when vcond_* do btw. - see the case I pointed out where it appears to have inconsistencies/wrong-code issues regarding ordered compare support. But see above (I do not know the powerpc ISA). A vcond can usually be emulated by vec_cmp plus masking. 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. Richard. > > Thanks, David
