<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    On 2016-06-14 17:34, Daniel Berlin wrote:<br>
    <blockquote
cite="mid:CAF4BwTWeTeMkzQp-KGGsPg0-4z0nyXoY7OmQUGHSKivaxt34AQ@mail.gmail.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
      <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
                moz-do-not-send="true"
                href="mailto:henric.karlsson@ericsson.com"
                target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:henric.karlsson@ericsson.com">henric.karlsson@ericsson.com</a></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 moz-do-not-send="true"
                  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>
    </blockquote>
    Ok, sounds like the DSE algorithm has room for improvements. And
    that it's better to keep the patch as it is, which shouldn't change 
    the current behavior, but make output from -g the same.<br>
  </body>
</html>