Mike Stump <mikest...@comcast.net> writes:
> On Oct 15, 2015, at 6:18 AM, Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>> Stripping unnecessary sign ops at the gimple level means that we're
>> no longer able to optimise:
>>
>> if (cos(y<10 ? -fabs(x) : tan(x<20 ? -x : -fabs(y)))
>> != cos(y<10 ? x : tan(x<20 ? x : y)))
>> link_error ();
>>
>> because we're currently not able to fold away the equality in:
>>
>> int
>> f1 (double x, double y)
>> {
>> double z1 = __builtin_cos(y<10 ? x : __builtin_tan(x<20 ? x : y));
>> double z2 = __builtin_cos(y<10 ? x : __builtin_tan(x<20 ? x : y));
>> return z1 == z2;
>> }
>>
>> The missed fold is being tracked as PR 67975. This patch splits the
>> test out into a separate file so that we can XFAIL it until the PR
>> is fixed.
>
> So, with my peanut gallery hat on, I’m not a fan of this sort of thing.
> If others want to do this though, I’d let that be their call.
>
> As a counter example, I’d be fine with #if 0ing the tests that don’t
> work, and putting in the PR the reserve of that patch to remove the
> added #if 0 when that PR is fixed. The test case remains complete, and
> once the PR is fixed, the test suite will be as it was. Having tests
> that we know fail doesn’t enhance the quality of the compiler any. It
> is a nice tracking mechanism, if no PR exists, but, when a PR exists, we
> don’t need the test case to track the issue, as the PR will. In
> general, adding #if 0 to tests is undesirable, but, when one cannot
> toggle subtests easily or mark subtest failures easily, I prefer it over
> splitting.
I can see that argument if people are only taking work items from
the PR database. But it's possible (likely even) that people will
independently find a problem like this and just fix it, if the missed
optimisation happens to be important to them. I don't think they
should then have to trawl the PR database to see which PRs their patch
fixes. XFAILs are a nice automated way of making it obvious.
(The patch that adds the XFAIL also adds a comment pointing to the
associated PR.)
Thanks,
Richard
