<div dir="ltr"><div class="gmail_extra">(correct, none of the below has anything to do with this patch)</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 15, 2015 at 4:58 PM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>So, I'm not sold on this being in the verifier at all.
              But that seems quite likely a separate discussion. </div>
          </div>
        </div>
      </div>
    </blockquote></span>
    Why?  Feel free to forward this to llvm-dev depending on the answer
    and it's length.<br>
    <br>
    Being able to state things like: "statepoints in functions with GC X
    only relocate values in an address space X expects" seems to be
    entirely reasonable and desirable to me.</blockquote><div><br></div><div>Absolutely, but I don't think it belongs in the IR verifier. The verifier needs to be extremely light weight and I don't think it makes sense to check these kinds of constraints there. The contract of the verifier is that it validates those properties which you should feel free to assert() on when writing other passes. I don't think we should add such properties lightly.</div><div><br></div><div>But of course it makes sense to check these properties. One way is through the verifyAnalysis functionality in the old pass manager (and there will be some largely-automated analog in the new one). This allows an analysis to check that its analysis specific invariants continue to hold. Another strategy is the (somewhat poorly named) Lint pass. The idea is that for some invariants it may make more sense to add a custom pass which does nothing other than check and diagnose violations of an invariant.</div><div><br></div><div>If checking this invariant in other ways were really hard to do, then maybe it would make sense in the verifier. But I'm not seeing anything like that at this point.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">  <br><span class="">
    <br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <div>However, even if it is in the verifier, I think it
              should still be an analysis pass, and the verifier should
              explicitly construct it. This is how the verifier uses the
              dominator trees. The nice thing is that it keeps the
              verifier from depending on the current state of an
              analysis pass which might hide actual errors in the IR. So
              the verifier just directly computes the GCStrategy, and
              then you have an analysis that provides access to a nicely
              cached one.</div>
          </div>
        </div>
      </div>
    </blockquote></span>
    So, first of all, the Verifier does not use the analysis pass
    mechanism.  It natively constructs and recalculates a DominatorTree
    which would normally be obtained through the DomTreeWrapperAnalysis
    pass.  Is this what you meant?<br></blockquote><div><br></div><div>To an extent.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
    <br>
    The analogous thing would be a GCStrategyCache and a wrapping
    GCStrategyCacheWrapper pass.  I have no objection to doing this if
    that's what the community would prefer, but I believe this is
    undesirable.  My reasoning is:<br>
    1) There is no possibility of invalidation.  A GCStrategy is
    immutable and can only be changed at compile time (of LLVM itself).</blockquote><div><br></div><div>There are actually quite a few things that have no invalidation criteria but I think make sense being exposed via the pass machinery. TargetLibraryInfo is an example I've just been touching.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
    2) If we have multiple instances of GCStrategyCache, we have
    multiple copies of each GCStrategy floating around with different
    lifetimes.  This seems potentially confusing at best.  (Particularly
    for any out of tree users... I'd really like the lifetime model to
    be simple.)<br></blockquote><div><br></div><div>I don't think this is necessary. You could easily have these be strict lazily created singletons.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
    3) We're adding an IMHO utterly unnecessary abstraction which only
    increases the complexity of an already fairly complex mechanism.  I
    see no benefit here.  </blockquote></div><br>I think there is a significant benefit to separating the concerns of GC strategy from the concerns of the core IR. I also clearly don't think the complexity is going to be that large.</div></div>