[llvm] b32bae6 - [Test] Add a unit test exposing lack of SCEV invalidation in LICM during code hoisting. NFC.

Serguei Katkov via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 31 03:17:33 PDT 2019


Author: Serguei Katkov
Date: 2019-10-31T17:16:57+07:00
New Revision: b32bae6f760896b0e285fb6967061e8cd281a8c1

URL: https://github.com/llvm/llvm-project/commit/b32bae6f760896b0e285fb6967061e8cd281a8c1
DIFF: https://github.com/llvm/llvm-project/commit/b32bae6f760896b0e285fb6967061e8cd281a8c1.diff

LOG: [Test] Add a unit test exposing lack of SCEV invalidation in LICM during code hoisting. NFC.

This unit test exposes a bug in LICM: when it hoists instructions it doesn't invalidate SCEV accordingly.
Similar test exposing lack of SCEV invalidation during code sinking will be submitted as a follow-up change.

Patch Author: Daniil Suchkov
Reviewers: mkazantsev, asbirlea, reames
Reviewed By: asbirlea
Subscribers: mgorny, javed.absar, llvm-commits
Differential Revision: https://reviews.llvm.org/D69369

Added: 
    llvm/unittests/Transforms/Scalar/LICMTest.cpp

Modified: 
    llvm/unittests/Transforms/Scalar/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/llvm/unittests/Transforms/Scalar/CMakeLists.txt b/llvm/unittests/Transforms/Scalar/CMakeLists.txt
index 3d01d5c53341..eaa2c4eb27a1 100644
--- a/llvm/unittests/Transforms/Scalar/CMakeLists.txt
+++ b/llvm/unittests/Transforms/Scalar/CMakeLists.txt
@@ -2,15 +2,19 @@ set(LLVM_LINK_COMPONENTS
   Analysis
   AsmParser
   Core
+  Passes
   Support
   ScalarOpts
   TransformUtils
   )
 
 add_llvm_unittest(ScalarTests
+  LICMTest.cpp
   LoopPassManagerTest.cpp
   )
 
+target_link_libraries(ScalarTests PRIVATE LLVMTestingSupport)
+
 # Workaround for the gcc 6.1 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80916.
 if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0 AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0)
   set_source_files_properties(LoopPassManagerTest.cpp PROPERTIES COMPILE_FLAGS -Wno-unused-function)

diff  --git a/llvm/unittests/Transforms/Scalar/LICMTest.cpp b/llvm/unittests/Transforms/Scalar/LICMTest.cpp
new file mode 100644
index 000000000000..91a9b10815a3
--- /dev/null
+++ b/llvm/unittests/Transforms/Scalar/LICMTest.cpp
@@ -0,0 +1,95 @@
+//===- LICMTest.cpp - LICM unit tests -------------------------------------===//
+//
+// 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/Analysis/ScalarEvolution.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Passes/PassBuilder.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Testing/Support/Error.h"
+#include "llvm/Transforms/Scalar/LICM.h"
+#include "gtest/gtest.h"
+
+namespace llvm {
+
+TEST(LICMTest, TestSCEVInvalidationOnHoisting) {
+  LLVMContext Ctx;
+  ModulePassManager MPM;
+  PassBuilder PB;
+  LoopAnalysisManager LAM;
+  FunctionAnalysisManager FAM;
+  CGSCCAnalysisManager CGAM;
+  ModuleAnalysisManager MAM;
+
+  PB.registerModuleAnalyses(MAM);
+  PB.registerCGSCCAnalyses(CGAM);
+  PB.registerFunctionAnalyses(FAM);
+  PB.registerLoopAnalyses(LAM);
+  PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
+
+  StringRef PipelineStr = "require<opt-remark-emit>,loop(licm)";
+  ASSERT_THAT_ERROR(PB.parsePassPipeline(MPM, PipelineStr), Succeeded());
+
+  SMDiagnostic Error;
+  StringRef Text = R"(
+    define void @foo(i64* %ptr) {
+    entry:
+      br label %loop
+
+    loop:
+      %iv = phi i64 [ 0, %entry ], [ %iv.inc, %loop ]
+      %n = load i64, i64* %ptr, !invariant.load !0
+      %iv.inc = add i64 %iv, 1
+      %cmp = icmp ult i64 %iv.inc, %n
+      br i1 %cmp, label %loop, label %exit
+
+    exit:
+      ret void
+    }
+
+    !0 = !{}
+  )";
+
+  std::unique_ptr<Module> M = parseAssemblyString(Text, Error, Ctx);
+  ASSERT_TRUE(M);
+  Function *F = M->getFunction("foo");
+  ScalarEvolution &SE = FAM.getResult<ScalarEvolutionAnalysis>(*F);
+  BasicBlock &EntryBB = F->getEntryBlock();
+  BasicBlock *LoopBB = EntryBB.getUniqueSuccessor();
+
+  // Select `load i64, i64* %ptr`.
+  Instruction *IBefore = LoopBB->getFirstNonPHI();
+  // Make sure the right instruction was selected.
+  ASSERT_TRUE(isa<LoadInst>(IBefore));
+  // Upon this query SCEV caches disposition of <load i64, i64* %ptr> SCEV.
+  ASSERT_EQ(SE.getBlockDisposition(SE.getSCEV(IBefore), LoopBB),
+            ScalarEvolution::BlockDisposition::DominatesBlock);
+
+  MPM.run(*M, MAM);
+
+  // Select `load i64, i64* %ptr` after it was hoisted.
+  Instruction *IAfter = EntryBB.getFirstNonPHI();
+  // Make sure the right instruction was selected.
+  ASSERT_TRUE(isa<LoadInst>(IAfter));
+
+  ScalarEvolution::BlockDisposition DispositionBeforeInvalidation =
+      SE.getBlockDisposition(SE.getSCEV(IAfter), LoopBB);
+  SE.forgetValue(IAfter);
+  ScalarEvolution::BlockDisposition DispositionAfterInvalidation =
+      SE.getBlockDisposition(SE.getSCEV(IAfter), LoopBB);
+
+  // If LICM have properly invalidated SCEV,
+  //   1. SCEV of <load i64, i64* %ptr> should properly dominate the "loop" BB,
+  //   2. extra invalidation shouldn't change result of the query.
+  // FIXME: these values should be equal!
+  EXPECT_NE(DispositionBeforeInvalidation,
+            ScalarEvolution::BlockDisposition::ProperlyDominatesBlock);
+  // FIXME: these values should be equal!
+  EXPECT_NE(DispositionBeforeInvalidation, DispositionAfterInvalidation);
+}
+}


        


More information about the llvm-commits mailing list