Re: [PATCH v2 1/8] Remove single-store shortcut

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

On Thu, Aug 10, 2017 at 1:01 PM, Christopher Li <sparse@chrisli.org> wrote:
>
> The reason this short cut is wrong is that, it is not  about
> how many store we have (single vs multiple).  It is about
> weather all the usage(load) has one single store
> immediate dominates it. If it does, even exist of other stores,
> this optimization can still be done.

More exactly, this shortcut is wrong because it is based on
the assumption that if there is a single store, then this store
must necessary dominates all the loads.
This assumption is, of course, wrong when the var is not
explicitly defined.

> The question would be if we have easy and quick way to
> detect that all the load are immediate dominated by the
> one single store.

That's what the general case does.

> If that condition holds, we can still do the "short-cut".

Only for limited special cases. It's not worth, forget about it.

Anyway, it's very temporary because the code still put the
phi-nodes at wrong places.
This simplify_one_symbol() is replaced by the new SSA construction
branch.

Believe it or not but this placement of phi-nodes is a much bigger
issue than those undefined vars. For example, on the kernel,
this patch has exactly zero (visible) effect. But every non-trivial
piece of code is plainly very wrong because of those bad phi-nodes.

> BTW, I consider this patch can separate out from the other
> patch in this series meaning wise.

Yes, it's what I said :"this patch and the preceding one
(because it's needed for the test case)".

> Please let me know if you want to include this in the RC5.
> Give me a proper git pull address. Topic branch is fine.

Yes, I'll do later.
(if you want just for testing, please take the ref in the cover letter
and drop the top patches. Unless comments from you, I don't
have the intention to change something to these two patches)

Regards,
-- Luc