<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <p>I've posted a new version which hopefully addresses all the
      raised issues.  Please take another look when you get a chance.</p>
    <p>Thanks,</p>
    <p>-Geoff<br>
    </p>
    <div class="moz-cite-prefix">On 5/4/2016 3:43 PM, George Burgess IV
      wrote:<br>
    </div>
    <blockquote
cite="mid:CAKh6zBFxW0-Yak77jYWMsW2bSLfxp-kiuO04_rNMB=exixpXgw@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div>
          <div><span style="font-size:12.8px">> I'm not convinced
              this is the right model in the new pass manager - it's</span><br>
          </div>
          <span style="font-size:12.8px">> essentially giving us two
            layers of caching, since the new PM already</span><br
            style="font-size:12.8px">
          <span style="font-size:12.8px">> computes the analysis on
            demand (when you first call getResult) and</span><br
            style="font-size:12.8px">
          <span style="font-size:12.8px">> caches that until its
            invalidated.</span><br>
          <div><span style="font-size:12.8px"><br>
            </span></div>
          <div><span style="font-size:12.8px">IIRC, the lazy pass was
              made before we decided to just compute everything eagerly
              all the time (which lets us provide the
              uses-point-to-their-actual-clobbering-defs guarantee). So,
              the name here is somewhat misleading. If it turns out that
              we don't need this at all, I think it would be fine to
              remove it. Otherwise, renaming it to just MemorySSAPass
              (or similar) would probably be best.</span></div>
        </div>
        <div><span style="font-size:12.8px"><br>
          </span></div>
        <div><span style="font-size:12.8px">> This is pretty weird in
            a printer pass. Does this mean we would need to</span><br
            style="font-size:12.8px">
          <span style="font-size:12.8px">> call buildMemorySSA in
            *any* user of the analysis pass?</span><span
            style="font-size:12.8px"><br>
          </span></div>
        <div><span style="font-size:12.8px"><br>
          </span></div>
        <div><span style="font-size:12.8px">Yeah, the API is a bit ugly
            here. :)</span></div>
        <div><span style="font-size:12.8px">buildMemorySSA is actually
            more like
            "buildMemorySSAOrGetTheWalkerIfWeveAlreadyBuiltMemorySSA".
            It may be cleaner to just have whatever pass that ends up
            wrapping MemorySSA hold a pointer to the walker that
            buildMemorySSA returns, and provide an accessor to that (in
            addition to allowing users to still grab MSSA, if they
            want).</span></div>
        <div class="gmail_extra"><br>
          <div class="gmail_quote">On Wed, May 4, 2016 at 10:40 AM,
            Justin Bogner <span dir="ltr"><<a moz-do-not-send="true"
                href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span>
            wrote:<br>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>Geoff
                Berry via llvm-commits <<a moz-do-not-send="true"
                  href="mailto:llvm-commits@lists.llvm.org"
                  target="_blank">llvm-commits@lists.llvm.org</a>>
                writes:<br>
                > gberry updated this revision to Diff 55623.<br>
                <br>
              </span>A couple of comments below. In short, I don't think
              this has quite the<br>
              right model. I've also cc'd Chandler for his thoughts.<br>
              <span><br>
                ><br>
                > <a moz-do-not-send="true"
                  href="http://reviews.llvm.org/D19664" rel="noreferrer"
                  target="_blank">http://reviews.llvm.org/D19664</a><br>
                ><br>
                > Files:<br>
                >   include/llvm/InitializePasses.h<br>
                >   include/llvm/Transforms/Utils/MemorySSA.h<br>
                >   lib/Passes/PassBuilder.cpp<br>
                >   lib/Passes/PassRegistry.def<br>
                >   lib/Transforms/Utils/MemorySSA.cpp<br>
                >   lib/Transforms/Utils/Utils.cpp<br>
                >   test/Transforms/Util/MemorySSA/assume.ll<br>
                >   test/Transforms/Util/MemorySSA/atomic-clobber.ll<br>
                >   test/Transforms/Util/MemorySSA/cyclicphi.ll<br>
                > 
                 test/Transforms/Util/MemorySSA/function-clobber.ll<br>
                > 
                 test/Transforms/Util/MemorySSA/function-mem-attrs.ll<br>
                >   test/Transforms/Util/MemorySSA/livein.ll<br>
                >   test/Transforms/Util/MemorySSA/load-invariant.ll<br>
                > 
                 test/Transforms/Util/MemorySSA/many-dom-backedge.ll<br>
                >   test/Transforms/Util/MemorySSA/many-doms.ll<br>
                >   test/Transforms/Util/MemorySSA/multi-edges.ll<br>
                > 
                 test/Transforms/Util/MemorySSA/multiple-backedges-hal.ll<br>
                > 
                 test/Transforms/Util/MemorySSA/multiple-locations.ll<br>
                >   test/Transforms/Util/MemorySSA/no-disconnected.ll<br>
                >   test/Transforms/Util/MemorySSA/optimize-use.ll<br>
                >   test/Transforms/Util/MemorySSA/phi-translation.ll<br>
                > 
                 test/Transforms/Util/MemorySSA/volatile-clobber.ll<br>
                ><br>
              </span>> Index:
              test/Transforms/Util/MemorySSA/volatile-clobber.ll<br>
              >
              ===================================================================<br>
              > ---
              test/Transforms/Util/MemorySSA/volatile-clobber.ll<br>
              > +++
              test/Transforms/Util/MemorySSA/volatile-clobber.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Ensures that volatile stores/loads count as
              MemoryDefs<br>
              ><br>
              > Index:
              test/Transforms/Util/MemorySSA/phi-translation.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/phi-translation.ll<br>
              > +++ test/Transforms/Util/MemorySSA/phi-translation.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              ><br>
              >  ; %ptr can't alias %local, so we should be able to
              optimize the use of %local to<br>
              >  ; point to the store to %local.<br>
              > Index: test/Transforms/Util/MemorySSA/optimize-use.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/optimize-use.ll<br>
              > +++ test/Transforms/Util/MemorySSA/optimize-use.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa -analyze
              -verify-memoryssa < %s 2>&1 | FileCheck %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              ><br>
              >  ; Function Attrs: ssp uwtable<br>
              >  define i32 @main() {<br>
              > Index:
              test/Transforms/Util/MemorySSA/no-disconnected.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/no-disconnected.ll<br>
              > +++ test/Transforms/Util/MemorySSA/no-disconnected.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa -analyze
              -verify-memoryssa < %s 2>&1 | FileCheck %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; This test ensures we don't end up with multiple
              reaching defs for a single<br>
              >  ; use/phi edge If we were to optimize defs, we would
              end up with 2=<br>
              > Index:
              test/Transforms/Util/MemorySSA/multiple-locations.ll<br>
              >
              ===================================================================<br>
              > ---
              test/Transforms/Util/MemorySSA/multiple-locations.ll<br>
              > +++
              test/Transforms/Util/MemorySSA/multiple-locations.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Checks that basicAA is doing some amount of
              disambiguation for us<br>
              ><br>
              > Index:
              test/Transforms/Util/MemorySSA/multiple-backedges-hal.ll<br>
              >
              ===================================================================<br>
              > ---
              test/Transforms/Util/MemorySSA/multiple-backedges-hal.ll<br>
              > +++
              test/Transforms/Util/MemorySSA/multiple-backedges-hal.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              ><br>
              >  ; hfinkel's case<br>
              >  ; [entry]<br>
              > Index: test/Transforms/Util/MemorySSA/multi-edges.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/multi-edges.ll<br>
              > +++ test/Transforms/Util/MemorySSA/multi-edges.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Makes sure we have a sane model if both successors
              of some block is the same<br>
              >  ; block.<br>
              > Index: test/Transforms/Util/MemorySSA/many-doms.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/many-doms.ll<br>
              > +++ test/Transforms/Util/MemorySSA/many-doms.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Testing many dominators, specifically from a
              switch statement in C.<br>
              ><br>
              > Index:
              test/Transforms/Util/MemorySSA/many-dom-backedge.ll<br>
              >
              ===================================================================<br>
              > ---
              test/Transforms/Util/MemorySSA/many-dom-backedge.ll<br>
              > +++
              test/Transforms/Util/MemorySSA/many-dom-backedge.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; many-dom.ll, with an added back-edge back into the
              switch.<br>
              >  ; Because people love their gotos.<br>
              > Index:
              test/Transforms/Util/MemorySSA/load-invariant.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/load-invariant.ll<br>
              > +++ test/Transforms/Util/MemorySSA/load-invariant.ll<br>
              > @@ -1,5 +1,6 @@<br>
              >  ; XFAIL: *<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Invariant loads should be considered live on
              entry, because, once the<br>
              >  ; location is known to be dereferenceable, the value
              can never change.<br>
              > Index: test/Transforms/Util/MemorySSA/livein.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/livein.ll<br>
              > +++ test/Transforms/Util/MemorySSA/livein.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  define void @F(i8*) {<br>
              >    br i1 true, label %left, label %right<br>
              >  left:<br>
              > Index:
              test/Transforms/Util/MemorySSA/function-mem-attrs.ll<br>
              >
              ===================================================================<br>
              > ---
              test/Transforms/Util/MemorySSA/function-mem-attrs.ll<br>
              > +++
              test/Transforms/Util/MemorySSA/function-mem-attrs.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Test that various function attributes give us sane
              results.<br>
              ><br>
              > Index:
              test/Transforms/Util/MemorySSA/function-clobber.ll<br>
              >
              ===================================================================<br>
              > ---
              test/Transforms/Util/MemorySSA/function-clobber.ll<br>
              > +++
              test/Transforms/Util/MemorySSA/function-clobber.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Ensuring that external functions without
              attributes are MemoryDefs<br>
              ><br>
              > Index: test/Transforms/Util/MemorySSA/cyclicphi.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/cyclicphi.ll<br>
              > +++ test/Transforms/Util/MemorySSA/cyclicphi.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              ><br>
              >  %struct.hoge = type { i32, %struct.widget }<br>
              >  %struct.widget = type { i64 }<br>
              > Index:
              test/Transforms/Util/MemorySSA/atomic-clobber.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/atomic-clobber.ll<br>
              > +++ test/Transforms/Util/MemorySSA/atomic-clobber.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Ensures that atomic loads count as MemoryDefs<br>
              ><br>
              > Index: test/Transforms/Util/MemorySSA/assume.ll<br>
              >
              ===================================================================<br>
              > --- test/Transforms/Util/MemorySSA/assume.ll<br>
              > +++ test/Transforms/Util/MemorySSA/assume.ll<br>
              > @@ -1,4 +1,5 @@<br>
              >  ; RUN: opt -basicaa -print-memoryssa
              -verify-memoryssa -analyze < %s 2>&1 | FileCheck
              %s<br>
              > +; RUN: opt -aa-pipeline=basic-aa
              -passes='print<memoryssa>' -verify-memoryssa
              -disable-output < %s 2>&1 | FileCheck %s<br>
              >  ;<br>
              >  ; Ensures that assumes are treated as not reading or
              writing memory.<br>
              ><br>
              > Index: lib/Transforms/Utils/Utils.cpp<br>
              >
              ===================================================================<br>
              > --- lib/Transforms/Utils/Utils.cpp<br>
              > +++ lib/Transforms/Utils/Utils.cpp<br>
              > @@ -34,7 +34,7 @@<br>
              >    initializeInstSimplifierPass(Registry);<br>
              >    initializeMetaRenamerPass(Registry);<br>
              >    initializeMemorySSALazyPass(Registry);<br>
              > -  initializeMemorySSAPrinterPassPass(Registry);<br>
              > + 
              initializeMemorySSAPrinterLegacyPassPass(Registry);<br>
              >  }<br>
              ><br>
              >  /// LLVMInitializeTransformUtils - C binding for
              initializeTransformUtilsPasses.<br>
              > Index: lib/Transforms/Utils/MemorySSA.cpp<br>
              >
              ===================================================================<br>
              > --- lib/Transforms/Utils/MemorySSA.cpp<br>
              > +++ lib/Transforms/Utils/MemorySSA.cpp<br>
              > @@ -46,15 +46,19 @@<br>
              >  STATISTIC(NumClobberCacheLookups, "Number of Memory
              SSA version cache lookups");<br>
              >  STATISTIC(NumClobberCacheHits, "Number of Memory SSA
              version cache hits");<br>
              >  STATISTIC(NumClobberCacheInserts, "Number of
              MemorySSA version cache inserts");<br>
              >
              -INITIALIZE_PASS_WITH_OPTIONS_BEGIN(MemorySSAPrinterPass,
              "print-memoryssa",<br>
              > -                                   "Memory SSA",
              true, true)<br>
              > +INITIALIZE_PASS_BEGIN(MemorySSAPrinterLegacyPass,
              "print-memoryssa",<br>
              > +                      "Memory SSA", true, true)<br>
              >  INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)<br>
              >  INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)<br>
              >  INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)<br>
              > -INITIALIZE_PASS_END(MemorySSAPrinterPass,
              "print-memoryssa", "Memory SSA", true,<br>
              > -                    true)<br>
              > +INITIALIZE_PASS_END(MemorySSAPrinterLegacyPass,
              "print-memoryssa", "Memory SSA",<br>
              > +                    true, true)<br>
              >  INITIALIZE_PASS(MemorySSALazy, "memoryssalazy",
              "Memory SSA", true, true)<br>
              ><br>
              > +static cl::opt<bool>
              VerifyMemorySSA("verify-memoryssa", cl::init(false),<br>
              > +                                     cl::Hidden,<br>
              > +                                     cl::desc("Run
              the Memory SSA verifier"));<br>
              > +<br>
              >  namespace llvm {<br>
              ><br>
              >  /// \brief An assembly annotator class to print
              Memory SSA information in<br>
              > @@ -677,45 +681,36 @@<br>
              >    dbgs() << "\n";<br>
              >  }<br>
              ><br>
              > -char MemorySSAPrinterPass::ID = 0;<br>
              > +char MemorySSAPrinterLegacyPass::ID = 0;<br>
              ><br>
              > -MemorySSAPrinterPass::MemorySSAPrinterPass() :
              FunctionPass(ID) {<br>
              > - 
              initializeMemorySSAPrinterPassPass(*PassRegistry::getPassRegistry());<br>
              >
              +MemorySSAPrinterLegacyPass::MemorySSAPrinterLegacyPass()
              : FunctionPass(ID) {<br>
              > + 
initializeMemorySSAPrinterLegacyPassPass(*PassRegistry::getPassRegistry());<br>
              >  }<br>
              ><br>
              > -void MemorySSAPrinterPass::releaseMemory() {<br>
              > +void MemorySSAPrinterLegacyPass::releaseMemory() {<br>
              >    // Subtlety: Be sure to delete the walker before
              MSSA, because the walker's<br>
              >    // dtor may try to access MemorySSA.<br>
              >    Walker.reset();<br>
              >    MSSA.reset();<br>
              >  }<br>
              ><br>
              > -void
              MemorySSAPrinterPass::getAnalysisUsage(AnalysisUsage
              &AU) const {<br>
              > +void
              MemorySSAPrinterLegacyPass::getAnalysisUsage(AnalysisUsage
              &AU) const {<br>
              >    AU.setPreservesAll();<br>
              >    AU.addRequired<AAResultsWrapperPass>();<br>
              >    AU.addRequired<DominatorTreeWrapperPass>();<br>
              >    AU.addPreserved<DominatorTreeWrapperPass>();<br>
              >    AU.addPreserved<GlobalsAAWrapperPass>();<br>
              >  }<br>
              ><br>
              > -bool MemorySSAPrinterPass::doInitialization(Module
              &M) {<br>
              > -  VerifyMemorySSA = M.getContext()<br>
              > -                        .getOption<bool,
              MemorySSAPrinterPass,<br>
              > -                                 
               &MemorySSAPrinterPass::VerifyMemorySSA>();<br>
              > +bool
              MemorySSAPrinterLegacyPass::doInitialization(Module
              &M) {<br>
              >    return false;<br>
              >  }<br>
              ><br>
              > -void MemorySSAPrinterPass::registerOptions() {<br>
              > -  OptionRegistry::registerOption<bool,
              MemorySSAPrinterPass,<br>
              > -                               
               &MemorySSAPrinterPass::VerifyMemorySSA>(<br>
              > -      "verify-memoryssa", "Run the Memory SSA
              verifier", false);<br>
              > -}<br>
              > -<br>
              > -void MemorySSAPrinterPass::print(raw_ostream
              &OS, const Module *M) const {<br>
              > +void MemorySSAPrinterLegacyPass::print(raw_ostream
              &OS, const Module *M) const {<br>
              >    MSSA->print(OS);<br>
              >  }<br>
              ><br>
              > -bool MemorySSAPrinterPass::runOnFunction(Function
              &F) {<br>
              > +bool
              MemorySSAPrinterLegacyPass::runOnFunction(Function &F)
              {<br>
              >    this->F = &F;<br>
              >    MSSA.reset(new MemorySSA(F));<br>
              >    AliasAnalysis *AA =
              &getAnalysis<AAResultsWrapperPass>().getAAResults();<br>
              > @@ -729,6 +724,36 @@<br>
              >    return false;<br>
              >  }<br>
              ><br>
              >
              +MemorySSAPrinterPass::MemorySSAPrinterPass(raw_ostream
              &OS) : OS(OS) {}<br>
              > +<br>
              > +PreservedAnalyses MemorySSAPrinterPass::run(Function
              &F,<br>
              > +                                           
              FunctionAnalysisManager &AM) {<br>
              > +  OS << "MemorySSA for function: " <<
              F.getName() << "\n";<br>
              > +<br>
              > +  auto &DT =
              AM.getResult<DominatorTreeAnalysis>(F);<br>
              > +  auto &AA = AM.getResult<AAManager>(F);<br>
              > +  MemorySSA &MSSA =
              AM.getResult<MemorySSALazyAnalysis>(F).getMSSA();<br>
              > +<br>
              > +  MSSA.buildMemorySSA(&AA, &DT);<br>
              <br>
              This is pretty weird in a printer pass. Does this mean we
              would need to<br>
              call buildMemorySSA in *any* user of the analysis pass?<br>
              <br>
              > +  MSSA.print(OS);<br>
              > +<br>
              > +  if (VerifyMemorySSA)<br>
              > +    MSSA.verifyMemorySSA();<br>
              <br>
              Just write a separate verify<memoryssa> pass, like
              we do for domtree and<br>
              the like. Having a cl::opt that modifies the print
              behaviour is<br>
              inflexible and awkward.<br>
              <br>
              > +<br>
              > +  return PreservedAnalyses::all();<br>
              > +}<br>
              > +<br>
              > +MemorySSALazyResults::MemorySSALazyResults(Function
              &F) {<br>
              > +  MSSA.reset(new MemorySSA(F));<br>
              > +}<br>
              > +<br>
              > +char MemorySSALazyAnalysis::PassID;<br>
              > +<br>
              > +MemorySSALazyResults<br>
              > +MemorySSALazyAnalysis::run(Function &F,
              AnalysisManager<Function> &AM) {<br>
              > +  return MemorySSALazyResults(F);<br>
              > +}<br>
              > +<br>
              >  char MemorySSALazy::ID = 0;<br>
              ><br>
              >  MemorySSALazy::MemorySSALazy() : FunctionPass(ID) {<br>
              > Index: lib/Passes/PassRegistry.def<br>
              >
              ===================================================================<br>
              > --- lib/Passes/PassRegistry.def<br>
              > +++ lib/Passes/PassRegistry.def<br>
              > @@ -74,6 +74,7 @@<br>
              >  FUNCTION_ANALYSIS("domfrontier",
              DominanceFrontierAnalysis())<br>
              >  FUNCTION_ANALYSIS("loops", LoopAnalysis())<br>
              >  FUNCTION_ANALYSIS("memdep",
              MemoryDependenceAnalysis())<br>
              > +FUNCTION_ANALYSIS("memoryssa",
              MemorySSALazyAnalysis())<br>
              >  FUNCTION_ANALYSIS("regions", RegionInfoAnalysis())<br>
              >  FUNCTION_ANALYSIS("no-op-function",
              NoOpFunctionAnalysis())<br>
              >  FUNCTION_ANALYSIS("scalar-evolution",
              ScalarEvolutionAnalysis())<br>
              > @@ -112,6 +113,7 @@<br>
              >  FUNCTION_PASS("print<demanded-bits>",
              DemandedBitsPrinterPass(dbgs()))<br>
              >  FUNCTION_PASS("print<domfrontier>",
              DominanceFrontierPrinterPass(dbgs()))<br>
              >  FUNCTION_PASS("print<loops>",
              LoopPrinterPass(dbgs()))<br>
              > +FUNCTION_PASS("print<memoryssa>",
              MemorySSAPrinterPass(dbgs()))<br>
              >  FUNCTION_PASS("print<regions>",
              RegionInfoPrinterPass(dbgs()))<br>
              >  FUNCTION_PASS("print<scalar-evolution>",
              ScalarEvolutionPrinterPass(dbgs()))<br>
              >  FUNCTION_PASS("reassociate", ReassociatePass())<br>
              > Index: lib/Passes/PassBuilder.cpp<br>
              >
              ===================================================================<br>
              > --- lib/Passes/PassBuilder.cpp<br>
              > +++ lib/Passes/PassBuilder.cpp<br>
              > @@ -62,6 +62,7 @@<br>
              >  #include "llvm/Transforms/Scalar/SROA.h"<br>
              >  #include "llvm/Transforms/Scalar/SimplifyCFG.h"<br>
              >  #include "llvm/Transforms/Scalar/Sink.h"<br>
              > +#include "llvm/Transforms/Utils/MemorySSA.h"<br>
              >  #include <type_traits><br>
              ><br>
              >  using namespace llvm;<br>
              > Index: include/llvm/Transforms/Utils/MemorySSA.h<br>
              >
              ===================================================================<br>
              > --- include/llvm/Transforms/Utils/MemorySSA.h<br>
              > +++ include/llvm/Transforms/Utils/MemorySSA.h<br>
              > @@ -561,7 +561,7 @@<br>
              >  protected:<br>
              >    // Used by Memory SSA annotater, dumpers, and
              wrapper pass<br>
              >    friend class MemorySSAAnnotatedWriter;<br>
              > -  friend class MemorySSAPrinterPass;<br>
              > +  friend class MemorySSAPrinterLegacyPass;<br>
              >    void verifyDefUses(Function &F) const;<br>
              >    void verifyDomination(Function &F) const;<br>
              ><br>
              > @@ -599,28 +599,63 @@<br>
              ><br>
              >  // This pass does eager building and then printing
              of MemorySSA. It is used by<br>
              >  // the tests to be able to build, dump, and verify
              Memory SSA.<br>
              > -class MemorySSAPrinterPass : public FunctionPass {<br>
              > +class MemorySSAPrinterLegacyPass : public
              FunctionPass {<br>
              >  public:<br>
              > -  MemorySSAPrinterPass();<br>
              > +  MemorySSAPrinterLegacyPass();<br>
              ><br>
              >    static char ID;<br>
              >    bool doInitialization(Module &M) override;<br>
              >    bool runOnFunction(Function &) override;<br>
              >    void releaseMemory() override;<br>
              >    void getAnalysisUsage(AnalysisUsage &AU) const
              override;<br>
              >    void print(raw_ostream &OS, const Module *M)
              const override;<br>
              > -  static void registerOptions();<br>
              >    MemorySSA &getMSSA() { return *MSSA; }<br>
              ><br>
              >  private:<br>
              > -  bool VerifyMemorySSA;<br>
              > -<br>
              >    std::unique_ptr<MemorySSA> MSSA;<br>
              >    // FIXME(gbiv): It seems that MemorySSA doesn't
              own the walker it returns?<br>
              >    std::unique_ptr<MemorySSAWalker> Walker;<br>
              >    Function *F;<br>
              >  };<br>
              ><br>
              > +/// \brief Printer pass for the \c MemorySSA.<br>
              > +class MemorySSAPrinterPass<br>
              > +    : public
              PassInfoMixin<MemorySSAPrinterPass> {<br>
              > +  raw_ostream &OS;<br>
              > +<br>
              > +public:<br>
              > +  explicit MemorySSAPrinterPass(raw_ostream
              &OS);<br>
              > +  PreservedAnalyses run(Function &F,
              AnalysisManager<Function> &AM);<br>
              > +};<br>
              > +<br>
              > +class MemorySSALazyResults {<br>
              > +public:<br>
              > +  MemorySSALazyResults(Function &F);<br>
              > +<br>
              > +  MemorySSA &getMSSA() {<br>
              > +    assert(MSSA);<br>
              > +    return *MSSA;<br>
              > +  }<br>
              > +<br>
              > +private:<br>
              > +  std::unique_ptr<MemorySSA> MSSA;<br>
              > +};<br>
              <br>
              I'm not convinced this is the right model in the new pass
              manager - it's<br>
              essentially giving us two layers of caching, since the new
              PM already<br>
              computes the analysis on demand (when you first call
              getResult) and<br>
              caches that until its invalidated.<br>
              <br>
              > +<br>
              > +/// An analysis that produces \c MemorySSA for a
              function.<br>
              > +///<br>
              > +/// This is essentially a no-op because the results
              are computed entirely<br>
              > +/// lazily.<br>
              > +class MemorySSALazyAnalysis<br>
              > +    : public
              AnalysisInfoMixin<MemorySSALazyAnalysis> {<br>
              > +  friend
              AnalysisInfoMixin<MemorySSALazyAnalysis>;<br>
              > +  static char PassID;<br>
              > +<br>
              > +public:<br>
              > +  typedef MemorySSALazyResults Result;<br>
              > +<br>
              > +  MemorySSALazyResults run(Function &F,
              AnalysisManager<Function> &AM);<br>
              > +};<br>
              > +<br>
              >  class MemorySSALazy : public FunctionPass {<br>
              >  public:<br>
              >    MemorySSALazy();<br>
              > Index: include/llvm/InitializePasses.h<br>
              >
              ===================================================================<br>
              > --- include/llvm/InitializePasses.h<br>
              > +++ include/llvm/InitializePasses.h<br>
              > @@ -219,7 +219,7 @@<br>
              >  void
              initializeMemDerefPrinterPass(PassRegistry&);<br>
              >  void
              initializeMemoryDependenceWrapperPassPass(PassRegistry&);<br>
              >  void initializeMemorySSALazyPass(PassRegistry&);<br>
              > -void
              initializeMemorySSAPrinterPassPass(PassRegistry&);<br>
              > +void
              initializeMemorySSAPrinterLegacyPassPass(PassRegistry&);<br>
              >  void
              initializeMergedLoadStoreMotionPass(PassRegistry &);<br>
              >  void initializeMetaRenamerPass(PassRegistry&);<br>
              >  void
              initializeMergeFunctionsPass(PassRegistry&);<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>
          <br>
        </div>
      </div>
    </blockquote>
    <br>
    <pre class="moz-signature" cols="72">-- 
Geoff Berry
Employee of Qualcomm Innovation Center, Inc.
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
</pre>
  </body>
</html>