Re: [PATCH v3 1/7] fix ptrlist corruption while killing unreachable BBs

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

On Mon, Jul 31, 2017 at 11:41 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Mon, Jul 31, 2017 at 4:36 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>>
>> Fix this by:
>> 1) refuse to emit the "crazy programmer" warning if there
>>    is a potential dead BB
>> 2) move kill_unreachable_bbs() in the main cleanup loop
>>    which will avoid nested ep->bbs loop.
>
> Great!

> Interesting.  So my reading is that, this is similar to the other
> alternative patch
> we discuss with different that:
> 1. move up kill_unreachable_bbs(ep) right after clean_up_insns(ep)
> 2. Avoid issue "crazy programmer" if we still have dead code to clean up.
>
> That sound very reasonable and I feel that is better than the previous version
> which eager to kill bbs. That is great.

Yes.
I just hope there isn't some other condition to would force again to
have to do the kill_unreachable_bbs() sooner.

> I really appreciate the change.

I thought you would.
The second difference is fundamental for correctness.
For the first, difference, I placed it between cleanup & CSE
because cleanup can create opportunities for dead BBs while
CSE doesn't and it's useless to do CSE on dead BBs.
So, it would be a small perf improvement.

Having some kill_unreachable_bbs() in the main loop was also
needed for the infinite loop problem (this in itself already stopped
the infinite loop with the wine code but the generated code was
very very wrong thus the need for the bb_depends_on() patch
which I think now fixes the root cause there).

I'll launch some stress testing tomorrow.

-- Luc