[PATCH] Fix PR82396: qsort comparator non-negative on sorted output

Maxim Kuvyrkov maxim.kuvyrkov@linaro.org
Thu Oct 5 17:34:00 GMT 2017


> On Oct 5, 2017, at 8:20 PM, Steve Ellcey <sellcey@cavium.com> wrote:
> 
> On Wed, 2017-10-04 at 13:24 +0000, Wilco Dijkstra wrote:
>> Richard Sandiford wrote:
>>> 
>>> 
>>> I don't think it's reasonable to commit this as obvious.  You said
>>> yourself in the covering message that "it doesn't at all restore
>>> the original behaviour since we no longer compare the base address".
>>> So even with the bootstrap failure, I think the patch needed review
>>> before going in.
>>> 
>>> Christophe's message doesn't change anything because you knew when you
>>> posted the patch that it fixed the failure.
>> Well my understanding was that it is OK to fix a bootstrap failure. I believe my
>> patch is trivial since it mostly removes redundant code. Also I took Jakub's
>> review as an OK as there were no technical objections. However since you
>> seem to disagree, I will revert it.
>> 
>> We have now had 5 days of no builds for a major target, which is a huge
>> inconvenience. So I don't think it is reasonable to wait any longer.
>> The alternative is to revert the original patch that caused the bootstrap failure
>> plus the patch(es) that unexpectedly changed the behaviour of the scheduler
>> (I don't think there was any testing as to what effect those had on the schedule).
>> 
>> So the question is who will do that and when?
>> 
>> Wilco
> 
> The aarch64 bootstrap is still broken.  I am adding the scheduler
> maintainers to the CC list on this email to see if one on them can
> review/approve Wilco's patch which was applied and then reverted.  If
> not, can one of the global maintainers revert the original patch that
> broke the bootstrap?

What the heck, I'll jump in as well.

I'm still working on analysis, but it appears to me that Alexander's patch (current state of trunk) fails qsort check due to not being symmetric for load/store analysis (write == 0 or write == 1) in comparisons with "irrelevant" instructions.  Wilco's patch does not seem to address that, and, possibly, makes the failure latent (I may be wrong here, it's late and I didn't finish analysis yet).

I'm currently bootstrapping the following patch (on aarch64-linux-gnu, arm-linux-gnueabihf will follow tomorrow), which (like Wilco's patch) seems to unbreak bootstrap, but is less invasive and preserves handling of multi_mem_insns.  Would please interested parties help me test it?

Comments?

--
Maxim Kuvyrkov
www.linaro.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-autopref_rank_for_schedule.patch
Type: application/octet-stream
Size: 2710 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20171005/fbc1776d/attachment.obj>


More information about the Gcc-patches mailing list