[NFC][LoopUtils] Added regression test for getEstimatedTripCount.

[ebrevnov]

Summary

As agreed in D71990 extracted regression test checnges to a separate review.

[ebrevnov]

@Ayal , would be nice if you find time to look at this patch since next two (accepted) patched depends on this one.

[fhahn]

Could you describe why the changes to run the tests via the pass managers is required? Wouldn't it be sufficient to use initialize the required analysis for the function and use that with getLoopEstimatedTripCount?

[ebrevnov]

In D72832#1826038, @fhahn wrote:

Could you describe why the changes to run the tests via the pass managers is required? Wouldn't it be sufficient to use initialize the required analysis for the function and use that with getLoopEstimatedTripCount?

They are not required. The thing is LoopUtilsTest.cpp didn't exist when I first developed my test. Later during one of the rebases I discovered that somebody else introduced a test with the same name. It didn't look right to me to do loop traversal in one test in two different ways. My personal preference was to use pass manager and I streamlined the code this way. Do you see any problems with that?

Optionally I can move my part to a separate file so they will be completely independent,.

[fhahn]

In D72832#1830747, @ebrevnov wrote:

In D72832#1826038, @fhahn wrote:

Could you describe why the changes to run the tests via the pass managers is required? Wouldn't it be sufficient to use initialize the required analysis for the function and use that with getLoopEstimatedTripCount?

They are not required. The thing is LoopUtilsTest.cpp didn't exist when I first developed my test. Later during one of the rebases I discovered that somebody else introduced a test with the same name. It didn't look right to me to do loop traversal in one test in two different ways. My personal preference was to use pass manager and I streamlined the code this way. Do you see any problems with that?

IMO pulling in the pass manager to run the tests adds quite a bit of additional complexity to the tests, without much benefit, unless I am missing something. I think it would be preferable to just instantiate the required classes directly. Also, I think overall there would be less code.

llvm/unittests/Transforms/Utils/LoopUtilsTest.cpp

 //===- LoopUtilsTest.cpp - Unit tests for LoopUtils -----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/LoopUtils.h"
+#include "llvm/Analysis/CGSCCPassManager.h"
+#include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/LoopPass.h"
 #include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/PassManager.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Passes/PassBuilder.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Transforms/Scalar/LoopPassManager.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
-static std::unique_ptr<Module> parseIR(LLVMContext &C, const char *IR) {
-  SMDiagnostic Err;
-  std::unique_ptr<Module> Mod = parseAssemblyString(IR, Err, C);
-  if (!Mod)
-    Err.print("LoopUtilsTests", errs());
-  return Mod;
-}
-
-static void run(Module &M, StringRef FuncName,
-                function_ref<void(Function &F, DominatorTree &DT,
-                                  ScalarEvolution &SE, LoopInfo &LI)>
-                    Test) {
-  Function *F = M.getFunction(FuncName);
-  DominatorTree DT(*F);
-  TargetLibraryInfoImpl TLII;
-  TargetLibraryInfo TLI(TLII);
-  AssumptionCache AC(*F);
-  LoopInfo LI(DT);
-  ScalarEvolution SE(*F, TLI, AC, DT, LI);
-  Test(*F, DT, SE, LI);
-}
-
-TEST(LoopUtils, DeleteDeadLoopNest) {
-  LLVMContext C;
-  std::unique_ptr<Module> M =
-      parseIR(C, "define void @foo() {\n"
-                 "entry:\n"
-                 "  br label %for.i\n"
-                 "for.i:\n"
-                 "  %i = phi i64 [ 0, %entry ], [ %inc.i, %for.i.latch ]\n"
-                 "  br label %for.j\n"
-                 "for.j:\n"
-                 "  %j = phi i64 [ 0, %for.i ], [ %inc.j, %for.j ]\n"
-                 "  %inc.j = add nsw i64 %j, 1\n"
-                 "  %cmp.j = icmp slt i64 %inc.j, 100\n"
-                 "  br i1 %cmp.j, label %for.j, label %for.k.preheader\n"
-                 "for.k.preheader:\n"
-                 "  br label %for.k\n"
-                 "for.k:\n"
-                 "  %k = phi i64 [ %inc.k, %for.k ], [ 0, %for.k.preheader ]\n"
-                 "  %inc.k = add nsw i64 %k, 1\n"
-                 "  %cmp.k = icmp slt i64 %inc.k, 100\n"
-                 "  br i1 %cmp.k, label %for.k, label %for.i.latch\n"
-                 "for.i.latch:\n"
-                 "  %inc.i = add nsw i64 %i, 1\n"
-                 "  %cmp.i = icmp slt i64 %inc.i, 100\n"
-                 "  br i1 %cmp.i, label %for.i, label %for.end\n"
-                 "for.end:\n"
-                 "  ret void\n"
-                 "}\n");
+namespace {
+struct LoopUtilsPass : public PassInfoMixin<LoopUtilsPass> {
+  template <typename T> LoopUtilsPass(T &&Arg) : Func(std::forward<T>(Arg)) {}
+
+  PreservedAnalyses run(Loop &L, LoopAnalysisManager &AM,
+      LoopStandardAnalysisResults &AR, LPMUpdater &U) {
+    return Func(L, AM, AR, U);
+  }
+
+  std::function<PreservedAnalyses(Loop &, LoopAnalysisManager &,
+                                  LoopStandardAnalysisResults &, LPMUpdater &)>
+  Func;
+};
+
+struct LoopUtilsTest : public testing::Test {
+
+  PassBuilder PB;
+  LoopAnalysisManager LAM;
+  FunctionAnalysisManager FAM;
+  CGSCCAnalysisManager CGAM;
+  ModuleAnalysisManager MAM;
+  LLVMContext Context;
+  std::unique_ptr<Module> M;
+
+  LoopUtilsTest() {
+    // Register all the basic analyses with the managers.
+    PB.registerModuleAnalyses(MAM);
+    PB.registerCGSCCAnalyses(CGAM);
+    PB.registerFunctionAnalyses(FAM);
+    PB.registerLoopAnalyses(LAM);
+    PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
+  }
+
+  void ParseAssembly(const char *IR) {
+    SMDiagnostic Error;
+    M = parseAssemblyString(IR, Error, Context);
+
+    std::string errMsg;
+    raw_string_ostream os(errMsg);
+    Error.print("", os);
+
+    // A failure here means that the test itself is buggy.
+    if (!M)
+      report_fatal_error(os.str().c_str());
+  }
+};
+
+TEST_F(LoopUtilsTest, DeleteDeadLoopNest) {
+  ParseAssembly("define void @foo() {\n"
 
-  run(*M, "foo",
-      [&](Function &F, DominatorTree &DT, ScalarEvolution &SE, LoopInfo &LI) {
-        assert(LI.begin() != LI.end() && "Expecting loops in function F");
-        Loop *L = *LI.begin();
-        assert(L && L->getName() == "for.i" && "Expecting loop for.i");
+
+  LoopPassManager LPM;
+  LPM.addPass(LoopUtilsPass([=](Loop &L, LoopAnalysisManager &AM,
+                                LoopStandardAnalysisResults &AR,
+                                LPMUpdater &U) -> PreservedAnalyses {
+    auto &F = *L.getHeader()->getParent();
+    auto &SE = AR.SE;
+    auto &LI = AR.LI;
+    auto &DT = AR.DT;
+
+    std::string LoopName = L.getName();
+    if (LoopName == "for.i") {
 
-        deleteDeadLoop(L, &DT, &SE, &LI);
+      deleteDeadLoop(&L, &DT, &SE, &LI);
 
-        assert(DT.verify(DominatorTree::VerificationLevel::Fast) &&
-               "Expecting valid dominator tree");
-        LI.verify(DT);
-        assert(LI.begin() == LI.end() &&
-               "Expecting no loops left in function F");
-        SE.verify();
+      assert(LI.begin() == LI.end() && "Expecting no loops left in function F");
 
-        Function::iterator FI = F.begin();
-        BasicBlock *Entry = &*(FI++);
-        assert(Entry->getName() == "entry" && "Expecting BasicBlock entry");
-        const BranchInst *BI = dyn_cast<BranchInst>(Entry->getTerminator());
-        assert(BI && "Expecting valid branch instruction");
-        EXPECT_EQ(BI->getNumSuccessors(), (unsigned)1);
-        EXPECT_EQ(BI->getSuccessor(0)->getName(), "for.end");
-      });
+
+      U.markLoopAsDeleted(L, LoopName);
+      return PreservedAnalyses::none();
+    }
+    return PreservedAnalyses::all();
+  }));
+
+  FunctionPassManager FPM;
+  FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM)));
+
+  ModulePassManager MPM;
+  MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
+
+  MPM.run(*M, MAM);
+}
+
+TEST_F(LoopUtilsTest, getLoopEstimatedTripCountTest) {
+  ParseAssembly(R"IR(
+      @a = dso_local global [1024 x i32] zeroinitializer, align 16
+      @b = dso_local global [1024 x i32] zeroinitializer, align 16
+      define dso_local void @_Z3foov() {
+      entry:
+        br label %for.body
+      for.cond.cleanup:                                 ; preds = %for.body
+        ret void
+      for.body:                                         ; preds = %for.body, %entry
+        %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+        %arrayidx = getelementptr inbounds [1024 x i32], [1024 x i32]* @b, i64 0, i64 %indvars.iv
+        %0 = load i32, i32* %arrayidx, align 4
+        %1 = trunc i64 %indvars.iv to i32
+        %mul = mul nsw i32 %0, %1
+        %arrayidx2 = getelementptr inbounds [1024 x i32], [1024 x i32]* @a, i64 0, i64 %indvars.iv
+        %2 = load i32, i32* %arrayidx2, align 4
+        %add = add nsw i32 %2, %mul
+        store i32 %add, i32* %arrayidx2, align 4
+        %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+        %exitcond = icmp eq i64 %indvars.iv.next, 1024
+        br i1 %exitcond, label %for.cond.cleanup, label %for.body, !prof !1}
+
+        !1 = !{!"branch_weights", i32 1, i32 15 })IR");
+
+
+  LoopPassManager LPM;
+  LPM.addPass(LoopUtilsPass([=](Loop &L, LoopAnalysisManager &AM,
+      LoopStandardAnalysisResults &AR, LPMUpdater &U) -> PreservedAnalyses {
+    auto TC = getLoopEstimatedTripCount(&L);
+    EXPECT_TRUE(TC.hasValue());
+    EXPECT_EQ(*TC, (unsigned)15);
+    return PreservedAnalyses::all();
+  }));
+
+  FunctionPassManager FPM;
+  FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM)));
+
+  ModulePassManager MPM;
+  MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
+
+  MPM.run(*M, MAM);
+}
 }