Re: sparse-next and preview of 0.5.1-rc5

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

On Fri, Jul 21, 2017 at 11:55:09PM -0400, Christopher Li wrote:
> Notice the "add.64      %r6 <- %r6, $4", that is very wrong.
> Not valid SSA form at all.

This is typical of situations where undefined pseudos are involved.
For example this is what Linus wrote on the subject some months ago:

	       and.32      %r3 <- %r4, $-65
	        or.32       %r4 <- %r3, $64
	
	which *initializes* the pseudo %r4 in the second instruction, but uses
	it in the first one. That's a classic pattern of uninitialized pseudos
	in sparse exactly because of the SSA logic.
	
	But it is all internally consistent, and the simplification is
	"correct". The simplification phase very much has a "garbage in,
	garbage out" model.

> I believe that is the one of the condition triggering the crazy programmer
> bug.

Of course, again this is explained in the original patch description.
The "crazy programmer" warning is all about this condition,
either in a single instruction or a chain producing such a cycle.

> It is just you pack the BB early so it does not
> trigger the "crazy programmer".

Yes, this is what was explained in the original patch description.

> I think your patch pack the BB early to avoid the "crazy programmer".
> That sounds more like a cover up rather than a proper fix up.

Like explained in the original patch description, this situation with
a self-defined pseudo can happen in unreachable code *even* if the
pseudo was in fact defined if:
- the definition was done in a loop header
- the loop header was deleted by insert_branch()
  * the definition is then removed, the pseudo become undefined
  * it shouldn't matter since all it's use is now in dead code
  * we don't know the that the loop is now dead code before
    kill_unreachable_bbs() is called.
  * therefore, kill_unreachable_bbs() must be called:
    - after a branch have been deleted
    - before simplify_one_memop() is called and is about to issue the
      "crazy programmer" warning 
      BECAUSE THE "crazy programmer" WARNING SHOULD NOT BE ISSUED
      IF THE CONCERNED CODE IS IN FACT DEAD CODE.

So, if you really prefer, you can call kill_unreachable_bbs() just before
the "crazy programmer" check instead of just after the branch delete
like it was done in my patch but calling kill_unreachable_bbs()
only after clean_up_insn() like done in your patch is wrong.

> If we have the proper SSA form, the "crazy programmer" shouldn't
> pop up at all.

See Linus explanation here above.


I found absolutely incredible the amount of time already wasted
on this issue and it is even not closed yet.
I feel really tired and totally disgusted by all this.

-- Luc