<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 14, 2016 at 2:02 AM, Henric Karlsson <span dir="ltr"><<a href="mailto:henric.karlsson@ericsson.com" target="_blank">henric.karlsson@ericsson.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Henric added a comment.<br>
<span class=""><br>
In <a href="http://reviews.llvm.org/D21076#456942" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21076#456942</a>, @dberlin wrote:<br>
<br>
> Note: bb->eraseFromParent now returns an iterator, so you can fix this by returning and using that instead  :)<br>
<br>
<br>
</span>This will change the behavior quite a bit, and then I think it's better to change it so we set BBI = BB.begin()<br>
<br>
We typically have this case:<br>
----------------------------<br>
<br>
Store -> A   (Dead)<br>
nextInst      (so iterator from eraseFromParent() will point here)<br>
...<br>
Store -> A  (current instruction)<br>
<br>
nextInst      (BBI typically points here)<br>
-----------------------------------------<br>
<br>
But this also made me realize that all this --BBI might also be unnecessary, just setting BBI to Inst->getIterator() might be good enough.<br>
On the other hand if we also remove loads and not only the store instruction, then we might open up for detecting new dead stores, so could be worth it to revisiting the basic block from the beginning again.<br></blockquote><div><br></div><div>Err, you if you process remove stores in the right order, you should never have to revisit the basic block.<br></div><div><br></div><div>If it's already iterating in reverse order, and erasing the later stores, then yeah, just keep it working.</div><div><br></div></div></div></div>