<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">On 12/07/2015 02:16 AM, James Molloy
      wrote:<br>
    </div>
    <blockquote
cite="mid:CALCTSA1YHhVGN8OmLnJuxboh3Zv1kZ93R743qGZMbEmmG3-0MQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">Hi Philip,
        <div><br>
        </div>
        <div>I've taken a closer look at this now. There are several
          problems with LVI:</div>
        <div><br>
        </div>
        <div>  1. getValueAt() is not as powerful as getValueAtEnd(). I
          think this is an oversight - it looks like the intention is
          that getValueAt() should be more powerful than getValueAtEnd()
          because it has the ability to use assume intrinsics and range
          metadata. However, if the range metadata and assume intrinsics
          give no answer (undefined), we don't fall back on our other
          analyses (getBlockValue()).</div>
      </div>
    </blockquote>
    This is a known (to me at least) problem.  I've got a series of
    patches stuck in review right now which will eventually fix this.<br>
    <blockquote
cite="mid:CALCTSA1YHhVGN8OmLnJuxboh3Zv1kZ93R743qGZMbEmmG3-0MQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div>  2. LVI does not look through select instructions.</div>
      </div>
    </blockquote>
    Again, I have a patch ready once review gets unblocked.  (To be
    clear, I think the next unblocking step is mine; the cycle time is
    just long enough to get it out of my active list.)<br>
    <blockquote
cite="mid:CALCTSA1YHhVGN8OmLnJuxboh3Zv1kZ93R743qGZMbEmmG3-0MQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div>  3. LVI does not handle loop PHIs at all.</div>
      </div>
    </blockquote>
    I'm not sure what you're describing here.  LVI can prove limited
    recursive conditions.  What are you trying to describe here?  Do you
    have a specific example case?<br>
    <blockquote
cite="mid:CALCTSA1YHhVGN8OmLnJuxboh3Zv1kZ93R743qGZMbEmmG3-0MQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div><br>
        </div>
        <div>I have patches for (1) and (2) and will send them for
          review irrespective of what happens with this review, as I
          think they're obviously good things to do. I think (3) is the
          real deal breaker - my motivating example has this chain of
          selects threaded over a loop, and LVI just cannot deal with
          loops at all.</div>
        <div><br>
        </div>
        <div>I think I recall someone (Jiangning Liu?) attempting to add
          support for loops to LVI, but having difficulty retaining the
          lattice structure and making sure it always converged.</div>
      </div>
    </blockquote>
    Yep, that would probably be challenging.  It does become easier once
    my "intersect" patch lands - since it can be a separable analysis
    which is then intersected - but it's not a small project.    <br>
    <blockquote
cite="mid:CALCTSA1YHhVGN8OmLnJuxboh3Zv1kZ93R743qGZMbEmmG3-0MQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div><br>
        </div>
        <div>So at the moment I'm not sure how else to implement this
          other than what I've already uploaded - even pushing the icmp
          queries up the use-def graph would eventually end up at the
          head of a loop - somewhere we need to do some analysis that
          can pass through loop PHIs.</div>
      </div>
    </blockquote>
    Ok, thank you for looking at the options.  I'm still a bit unhappy
    about the chosen approach, but with your explanation I see why you
    settled on the given approach.  I withdraw my objections.  <br>
    <blockquote
cite="mid:CALCTSA1YHhVGN8OmLnJuxboh3Zv1kZ93R743qGZMbEmmG3-0MQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div><br>
        </div>
        <div>Cheers,</div>
        <div><br>
        </div>
        <div>James</div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr">On Sat, 5 Dec 2015 at 19:34 James Molloy <<a
            moz-do-not-send="true" href="mailto:james@jamesmolloy.co.uk"><a class="moz-txt-link-abbreviated" href="mailto:james@jamesmolloy.co.uk">james@jamesmolloy.co.uk</a></a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0 0 0
          .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Philip,<br>
          <br>
          Thanks for taking a look!<br>
          <br>
          I agree that searching an arbitrary size value graph would be
          horrendously expensive. What I have implemented is to search
          through unlimited selects and phis (should be cheap, these
          aren't commonly chained deeply, see GetUnderlyingObjects), and
          will look through only a certain number of binary operators.
          The default depth is 1, which will go through one add but no
          more (configurable by the user). I think overall this should
          restrict the potential for bloat. <br>
          <br>
          I have a motivating example which is a chain of selects and
          phis in a loop. The loop part is important - it's what seems
          to confuse instcombine currently. What I can do is take a look
          to see what's causing CVP / LVI to not support this case and
          report back. Thanks for the hint!<br>
          <br>
          I'm not sure the last approach you suggested would work for
          the loop use case, although I haven't thought majorly hard
          about it just yet. <br>
          <br>
          Thanks again,<br>
          <br>
          James<br>
          <div class="gmail_quote">
            <div dir="ltr">On Fri, 4 Dec 2015 at 20:36, Philip Reames
              via llvm-commits <<a moz-do-not-send="true"
                href="mailto:llvm-commits@lists.llvm.org"
                target="_blank">llvm-commits@lists.llvm.org</a>>
              wrote:<br>
            </div>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">reames
              added a subscriber: reames.<br>
              reames added a comment.<br>
              <br>
              At a high level, I'm a bit unsure about the formulation
              chosen here.  I'm worried that seeking to find a set of
              constant values through a potentially large value graph
              might be expensive.<br>
              <br>
              A couple of specific questions:<br>
              <br>
              - The examples you've listed should be entirely handled by
              LazyValueInfo and either JumpThreading or CVP.  Do you
              have a strong reason InstCombine needs to perform the same
              optimization?<br>
              - Have you considered phrasing this as pushing the query
              (i.e. icmp) back along the inputs?  Doing this might a)
              let you terminate earlier or b) unswitch the comparison if
              the select is only used by the compare.<br>
              <br>
              <br>
              <a moz-do-not-send="true"
                href="http://reviews.llvm.org/D15232" rel="noreferrer"
                target="_blank">http://reviews.llvm.org/D15232</a><br>
              <br>
              <br>
              <br>
              _______________________________________________<br>
              llvm-commits mailing list<br>
              <a moz-do-not-send="true"
                href="mailto:llvm-commits@lists.llvm.org"
                target="_blank">llvm-commits@lists.llvm.org</a><br>
              <a moz-do-not-send="true"
                href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
                rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
            </blockquote>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </body>
</html>