[Attributor] Make use of analysis in the MustBeExecutedExplorer

[okura]

Summary

This commit was made to settle this issue on GitHub.
I added analysis getters for LoopInfo, DominatorTree, and PostDominatorTree. And I added a test to show an improvement of the deduction of dereferenceable attribute.

[sstefan1]

llvm/include/llvm/Transforms/IPO/Attributor.h

566

Maybe I misunderstood something, but isn't this supposed to return existing analysis results instead of creating new ones?

[sstefan1]

llvm/include/llvm/Transforms/IPO/Attributor.h

566

my bad, sorry

[uenoku]

I think Stefan is correct. AnalysisGetter provides such functionality.

[okura]

Thank you both of you. I will change to get an analysis through AnalysisGetter.

[okura]

I changed to get analyses through AnalysisGetter.

[uenoku]

test?

[okura]

fix directive in a test

[uenoku]

LGTM

llvm/include/llvm/Transforms/IPO/Attributor.h

568

nit: //LIGetter -> /* LIGetter*/
https://llvm.org/docs/CodingStandards.html#comment-formatting
Other arguments are as well.

[okura]

fixed comment style

[okura]

llvm/include/llvm/Transforms/IPO/Attributor.h

568

I'm sorry for not following the standards. I fixed them.

[jdoerfert]

We have no other test changes? Unfortunate.

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

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?

[baziotis]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

That makes sense. I tested locally and it doesn't pass, maybe I miss something too though.

[uenoku]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

Sorry, I have missed this test uses OPM. It doesn't pass locally.

[sstefan1]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

Could be that he didn't re-run the tests, since it should have worked with previous diff that didn't use AnalysisGetter.

[jdoerfert]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

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

[baziotis]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

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.

[okura]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

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.

[sstefan1]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

yes, add a new RUN line. You can take a look at align.ll for example

[okura]

add a new RUN line

[okura]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

I added it. I don't know what kind of name is appropriate for the label, so I use the default prefix.

[jdoerfert]

One Nit, LGTM (assuming it passes all tests).

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

Make the prefix ATTRIBUTOR_CGSCC_NPM

[okura]

change prefix name

[okura]

llvm/test/Transforms/Attributor/dereferenceable-2.ll

400

I changed the prefix.

[jdoerfert]

I want to upstream but to attribute this to you I need "Firstname Lastname <email>" from you

[okura]

In D76378#1956332, @jdoerfert wrote:

I want to upstream but to attribute this to you I need "Firstname Lastname <email>" from you

Shinji Okumura [e-mail]

llvm/include/llvm/Transforms/IPO/Attributor.h

 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SCCIterator.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/InlineCost.h"
 #include "llvm/Analysis/LazyCallGraph.h"
+#include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/MustExecute.h"
+#include "llvm/Analysis/PostDominators.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Transforms/Utils/CallGraphUpdater.h"
 
 namespace llvm {

 /// Attributor::registerAA(...), need more information, especially if it is not
 /// reusable, it is advised to inherit from the InformationCache and cast the
 /// instance down in the abstract attributes.
 struct InformationCache {
   InformationCache(const Module &M, AnalysisGetter &AG,
                    SetVector<Function *> *CGSCC)
       : DL(M.getDataLayout()),
         Explorer(/* ExploreInterBlock */ true, /* ExploreCFGForward */ true,
-                 /* ExploreCFGBackward */ true),
+                 /* ExploreCFGBackward */ true,
+                 // LIGetter
+                 [](const Function &F) {
+                   return new LoopInfo(
+                       *(new DominatorTree(const_cast<Function &>(F))));
+                 },
+                 // DTGetter
+                 [](const Function &F) {
+                   return new DominatorTree(const_cast<Function &>(F));
+                 },
+                 // PDTGetter
+                 [](const Function &F) {
+                   return new PostDominatorTree(const_cast<Function &>(F));
+                 }),
         AG(AG), CGSCC(CGSCC) {}
 
   /// A map type from opcodes to instructions with this opcode.
   using OpcodeInstMapTy = DenseMap<unsigned, SmallVector<Instruction *, 32>>;
 
   /// Return the map that relates "interesting" opcodes with all instructions
   /// with that opcode in \p F.
   OpcodeInstMapTy &getOpcodeInstMapForFunction(const Function &F) {

llvm/test/Transforms/Attributor/dereferenceable-2.ll

 
 define void @different_size2(i32* %arg) {
 ; ATTRIBUTOR-LABEL: @different_size2(i32* nocapture nofree nonnull writeonly dereferenceable(8) %arg)
   store i32 0, i32* %arg
   %arg-cast = bitcast i32* %arg to double*
   store double 0.000000e+00, double* %arg-cast
   ret void
 }
+
+; Make use of MustBeExecuted Explorer
+;
+; [CFG]
+; entry
+;  / \
+; l1 l2
+; | X |
+; l3 l4
+;  \ /
+;  l5
+;  / \
+; l6 l7
+;  \ /
+;  end
+; According to the above CFG, we can see that instructions in l5 Block must be executed.
+; Therefore, %p must be dereferenced.
+;
+; CHECK: define i32 @require_cfg_analysis(i32 %c, i32* dereferenceable(4) %p)
+define i32 @require_cfg_analysis(i32 %c, i32* %p) {
+  %tobool1 = icmp eq i32 %c, 0
+  br i1 %tobool1, label %l1, label %l2
+l1:                                       
+  %tobool2 = icmp eq i32 %c, 1
+  br i1 %tobool2, label %l3, label %l4
+l2:  
+  %tobool3 = icmp eq i32 %c, 2
+  br i1 %tobool3, label %l3, label %l4
+l3:
+  br label %l5
+l4:
+  br label %l5
+l5:
+  %tobool4 = icmp eq i32 %c, 4
+  br i1 %tobool4, label %l6, label %l7
+l6:
+  store i32 0, i32* %p
+  br label %end
+l7:
+  store i32 1, i32* %p
+  br label %end
+end:
+  ret i32 1
+}