On 02/06/2016 12:08 PM, David Edelsohn wrote:
Normally I'd say that if it was approved before, then it's still good to go
since there haven't been major conceptual changes in this code since the
patch was originally written and now.
However, in this instance the patch had been reported to cause problems on
AIX, problems that we can't reproduce now -- which makes me want to be more
cautious. Was it a problem with the patch, or some other latent issue -- we
don't know at this point.
So I think the way to go is to apply this patch on top of r219827 where it
caused the AIX failure. Then bootstrap on aix and determine the root cause
of of the AIX bootstrap failure. If it's this patch, then update the patch
as needed. If the patch is just exposing a latent bug elsewhere, we should
evaluate whether or not that latent but has been fixed or not before
applying this fix to the trunk.
It's considerably more work, but ISTM it's the right thing to do.
I'm on the fence about this patch. I definitely don't think that it
should be merged for GCC 6.
If the patch were to be proposed during Stage 1 for GCC 7 and had not
caused bootstrap problems for AIX, no one would have any question.
The problem is we don't know if the patch exposed a latent bug that
independently was fixed after the patch was reverted or if the patch
still contains a bug that has been rendered latent by another change.
Another approach to track down the cause would be to bisect which
patch fixed the bootstrap failure if the patch had not been reverted.
Yes, that would be a good approach as well. The concern here would be
that without doing the root cause analysis, bisection may just find a
patch which made the issue go latent. To be sure we still have to do
some root cause analysis.
Given this fixes a regression, I'm still open to incorporating the
patch, but we've got to know what went wrong when the patch was
previously applied and that whatever that problem was got fixed.
Jeff
