On Thu, Aug 10, 2017 at 1:01 PM, Christopher Li <sparse@xxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
