<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <div class="moz-cite-prefix">On 07/17/2014 01:51 PM, Chandler
      Carruth wrote:<br>
    </div>
    <blockquote
cite="mid:CAGCO0Ki77WzzG86xg+NMH_62sRG7c0SCFdGoagUb_whURrY9UQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_extra"><br>
          <div class="gmail_quote"><br>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div id=":599" class="a3s" style="overflow:hidden"> 
                2. Would adding a canonicalization of if(c) {
                unreachable } to llvm.invariant(c) would be worthwhile?<br>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>There was a long and painful attempt to implement
              invariants based on the branch-to-unreachable pattern. It
              didn't work. I don't expect these patterns to show up
              often organically and to go away too soon in the optimizer
              to be useful. The whole point of 'llvm.invariant' instead
              of the if construct is to distinguish between the case the
              optimizer should try to remove (a branch to unreachable)
              and the case the optimizer should try to preserve because
              of some specific utility. We shouldn't lose this important
              distinction.</div>
          </div>
        </div>
      </div>
    </blockquote>
    On first thought, I disagree.  I may not be understanding your point
    though.<br>
    <br>
    My understanding of the previous work was that it tried to use
    "branch-to-unreachable" as the canonical form.  This is inherently
    problematic in an IR based on basic blocks since it split basic
    blocks into many smaller chunks.  It might work out better if we
    used extended basic blocks, but we don't.  <br>
    <br>
    I don't see any harm in canonicalizing to "llvm.invariant" from
    "if(c) unreachable".  In either case, we remove the branch and can
    merge the basic blocks.  In the former, we preserve more information
    for later passes.  The only real downside is potentially keeping
    more Values alive and thus forcing the compiler to do more work.  <br>
    <br>
    Can you spell out your objections more?<br>
    <br>
    <br>
    <blockquote
cite="mid:CAGCO0Ki77WzzG86xg+NMH_62sRG7c0SCFdGoagUb_whURrY9UQ@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <div> </div>
            <br>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div id=":599" class="a3s" style="overflow:hidden">
                <br>
                 2. We might want to have passes precompute the
                Value->(set of Invariants) map, and update it as
                necessary instead of doing transitive-user searches [a
                suggestion by Chandler].<br>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>
              I think this is mostly just a small API tweak to make the
              code more maintainable. I'm specifically *not* imagining
              this as a formal analysis pass or anything of the sort.</div>
          </div>
        </div>
      </div>
    </blockquote>
    I would support this.  <br>
    <br>
    I'm not seeing the downside to an analysis pass.  Probably not worth
    implementing up front, but might be worthwhile down the road. <br>
    <br>
    Philip<br>
  </body>
</html>