[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

564

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

564

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

566

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

566

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

401

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

401

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

401

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

[sstefan1]

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

401

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

401

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

401

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

401

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

401

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

401

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

401

Make the prefix ATTRIBUTOR_CGSCC_NPM

[okura]

change prefix name

[okura]

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

401

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/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/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 {

 /// If custom abstract attributes, registered manually through
 /// 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),
+        Explorer(
+            /* ExploreInterBlock */ true, /* ExploreCFGForward */ true,
+            /* ExploreCFGBackward */ true,
+            /* LIGetter */
+            [&](const Function &F) { return AG.getAnalysis<LoopAnalysis>(F); },
+            /* DTGetter */
+            [&](const Function &F) {
+              return AG.getAnalysis<DominatorTreeAnalysis>(F);
+            },
+            /* PDTGetter */
+            [&](const Function &F) {
+              return AG.getAnalysis<PostDominatorTreeAnalysis>(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

 ; RUN: opt < %s -attributor --attributor-disable=false -S | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt < %s -passes=attributor --attributor-disable=false -S | FileCheck %s --check-prefix=ATTRIBUTOR_CGSCC_NPM
 ; Copied from Transforms/InferFunctionAttrs/dereferenceable.ll
 
 ; Determine dereference-ability before unused loads get deleted:
 ; https://bugs.llvm.org/show_bug.cgi?id=21780
 
 define <4 x double> @PR21780(double* %ptr) {
 ; ATTRIBUTOR-LABEL: @PR21780(double* nocapture nofree nonnull readonly align 8 dereferenceable(32) %ptr)
 

 
 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.
+;
+; ATTRIBUTOR_CGSCC_NPM-LABEL: 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
+}