[PATCH] D76378: [Attributor] Make use of analysis in the MustBeExecutedExplorer

[Shinji Okumura via Phabricator via llvm-commits]

okura marked an inline comment as done.
okura added inline comments.


================
Comment at: llvm/test/Transforms/Attributor/dereferenceable-2.ll:400
+  ret i32 1
+}
----------------
jdoerfert wrote:
> okura wrote:
> > sstefan1 wrote:
> > > okura wrote:
> > > > baziotis wrote:
> > > > > jdoerfert wrote:
> > > > > > sstefan1 wrote:
> > > > > > > uenoku wrote:
> > > > > > > > baziotis wrote:
> > > > > > > > > jdoerfert wrote:
> > > > > > > > > > TBH, I'm confused why this works. this test is only run in the old pass manager which doesn't support querying analysis passes (from the Attributor). Where am I wrong?
> > > > > > > > > That makes sense. I tested locally and it doesn't pass, maybe I miss something too though.
> > > > > > > > Sorry, I have missed this test uses OPM. It doesn't pass locally. 
> > > > > > > Could be that he didn't re-run the tests, since it should have worked with previous diff that didn't use `AnalysisGetter`.
> > > > > > FWIW, the solution here is to verify this works in the new pass manager ;), hence add a run line and different check lines for the new PM
> > > > > Just to note: To do that, replace `-attributor` with `-passes=attributor`. I think it's difficult to find it, I didn't know until @uenoku told me.
> > > > As @sstefan1 mentioned, I forgot to re-run tests, and it doesn't pass. I am sorry for my carelessness. I don't know much about the pass manager, so I'm not sure whether I have to add a new RUN line or replace it.
> > > yes, add a new RUN line. You can take a look at align.ll for example
> > I added it. I don't know what kind of name is appropriate for the label, so I use the default prefix.
> Make the prefix `ATTRIBUTOR_CGSCC_NPM`
I changed the prefix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76378/new/

https://reviews.llvm.org/D76378