<html>

  <head>

    <meta content="text/html; charset=ISO-8859-1"

      http-equiv="Content-Type">

  </head>

  <body bgcolor="#FFFFFF" text="#000000">

    <div class="moz-cite-prefix">On 08/02/2013 04:47 PM, Eli Friedman

      wrote:<br>

    </div>

    <blockquote

cite="mid:CAJdarcHOZ3SvpzA-vOxHzRUNGxHt2=L9KKNZ-VP87ppD8RReqA@mail.gmail.com"

      type="cite">

      <pre wrap="">On Thu, Aug 1, 2013 at 1:34 PM, Matt Arsenault

<a class="moz-txt-link-rfc2396E" href="mailto:Matthew.Arsenault@amd.com"><Matthew.Arsenault@amd.com></a> wrote:

</pre>

      <blockquote type="cite">

        <pre wrap="">Turn check / insert cast into assertions that the sizes are the same. Add some more tests to try to break it. These paths seemed to not be tested with a datalayout. This checks if it needs to cast GEP indices, but at this point they should have already been converted to the pointer sized integer.

<a class="moz-txt-link-freetext" href="http://llvm-reviews.chandlerc.com/D1261">http://llvm-reviews.chandlerc.com/D1261</a>

Files:

  lib/Transforms/InstCombine/InstCombineCompares.cpp

  test/Transforms/InstCombine/getelementptr.ll

  test/Transforms/InstCombine/icmp.ll

  test/Transforms/InstCombine/load-cmp.ll

</pre>

      </blockquote>

      <pre wrap="">

I'm pretty sure this patch is wrong.  instcombine will always

eventually transform GEPs in the way you expect, and in trivial cases,

we will visit the GEP before the icmp, but I think we can visit the

icmp first in more complicated cases.

-Eli

</pre>

    </blockquote>

    I've looked at the order InstCombine tries to do things again, and

    I'm more confident now that this situation can't happen. The

    initially created

    worklists reversed so the pre-existing instructions are visited in

    order, so the simple case is always fine.<big> </big>A cmp using a

    loaded value from the result of a GEP will necessarily be visited

    after the GEP is visited and transformed. <br>

    <br>

    The order gets a bit confusing when an instruction is replaced. The

    users of the instructions are added to the worklist after the

    result, so they will be visited before the replaced instruction and

    other initial instructions not yet visited. This would be a problem

    if the cmp instruction were a user of a replaced instruction. The

    cmps in question must have a constant operand, and then the pointer

    from the GEP. Therefore, it can't be a user of some other

    instruction that could be replaced that would cause it to be visited

    earlier. Items are also moved to the top of the worklist only if

    they aren't already in the map of seen instructions, so any

    instructions that existed before the InstCombine iteration began

    wouldn't be impacted and should be visited in the right order. This

    would be a problem if InstCombine introduced a new GEP with the

    wrong size indices that was used by a cmp, but it shouldn't be doing

    that.<br>

  </body>

</html>