From 08d064c8f1beb5495bd456b16a73bf5617a12b42 Mon Sep 17 00:00:00 2001 From: Johannes Doerfert Date: Wed, 28 Aug 2019 16:58:52 +0000 Subject: [PATCH] [Attributor] Regularly clear dependences to remove spurious ones As dependences between abstract attributes can become stale, e.g., if one was sufficient to imply another one at some point but it has since been wakened to the point it is not usable for the formerly implied one. To weed out spurious dependences, and thereby eliminate unneeded updates, we introduce an option to determine how often the dependence cache is cleared and recomputed during the fixpoint iteration. Note that the initial value was determined such that we see a positive result on our tests. Differential Revision: https://reviews.llvm.org/D63315 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370230 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Transforms/IPO/Attributor.h | 14 +++++++- lib/Transforms/IPO/Attributor.cpp | 35 ++++++++++++++++--- test/Transforms/FunctionAttrs/align.ll | 2 +- test/Transforms/FunctionAttrs/nonnull.ll | 2 +- .../Transforms/FunctionAttrs/noreturn_sync.ll | 2 +- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/include/llvm/Transforms/IPO/Attributor.h b/include/llvm/Transforms/IPO/Attributor.h index f8917bcd6fb..4b5d266d4d8 100644 --- a/include/llvm/Transforms/IPO/Attributor.h +++ b/include/llvm/Transforms/IPO/Attributor.h @@ -568,7 +568,15 @@ private: /// NOTE: The mechanics of adding a new "concrete" abstract attribute are /// described in the file comment. struct Attributor { - Attributor(InformationCache &InfoCache) : InfoCache(InfoCache) {} + /// Constructor + /// + /// \param InformationCache Cache to hold various information accessible for + /// the abstract attributes. + /// \param DepRecomputeInterval Number of iterations until the dependences + /// between abstract attributes are recomputed. + Attributor(InformationCache &InfoCache, unsigned DepRecomputeInterval) + : InfoCache(InfoCache), DepRecomputeInterval(DepRecomputeInterval) {} + ~Attributor() { DeleteContainerPointers(AllAbstractAttributes); } /// Run the analyses until a fixpoint is reached or enforced (timeout). @@ -775,6 +783,10 @@ private: /// The information cache that holds pre-processed (LLVM-IR) information. InformationCache &InfoCache; + /// Number of iterations until the dependences between abstract attributes are + /// recomputed. + const unsigned DepRecomputeInterval; + /// Functions, blocks, and instructions we delete after manifest is done. /// ///{ diff --git a/lib/Transforms/IPO/Attributor.cpp b/lib/Transforms/IPO/Attributor.cpp index d441012000e..be609a2b238 100644 --- a/lib/Transforms/IPO/Attributor.cpp +++ b/lib/Transforms/IPO/Attributor.cpp @@ -123,6 +123,11 @@ static cl::opt VerifyAttributor( "manifestation of attributes -- may issue false-positive errors"), cl::init(false)); +static cl::opt DepRecInterval( + "attributor-dependence-recompute-interval", cl::Hidden, + cl::desc("Number of iterations until dependences are recomputed."), + cl::init(4)); + /// Logic operators for the change status enum class. /// ///{ @@ -2548,12 +2553,25 @@ ChangeStatus Attributor::run() { SetVector Worklist; Worklist.insert(AllAbstractAttributes.begin(), AllAbstractAttributes.end()); + bool RecomputeDependences = false; + do { // Remember the size to determine new attributes. size_t NumAAs = AllAbstractAttributes.size(); LLVM_DEBUG(dbgs() << "\n\n[Attributor] #Iteration: " << IterationCounter << ", Worklist size: " << Worklist.size() << "\n"); + // If dependences (=QueryMap) are recomputed we have to look at all abstract + // attributes again, regardless of what changed in the last iteration. + if (RecomputeDependences) { + LLVM_DEBUG( + dbgs() << "[Attributor] Run all AAs to recompute dependences\n"); + QueryMap.clear(); + ChangedAAs.clear(); + Worklist.insert(AllAbstractAttributes.begin(), + AllAbstractAttributes.end()); + } + // Add all abstract attributes that are potentially dependent on one that // changed to the work list. for (AbstractAttribute *ChangedAA : ChangedAAs) { @@ -2575,6 +2593,10 @@ ChangeStatus Attributor::run() { if (AA->update(*this) == ChangeStatus::CHANGED) ChangedAAs.push_back(AA); + // Check if we recompute the dependences in the next iteration. + RecomputeDependences = (DepRecomputeInterval > 0 && + IterationCounter % DepRecomputeInterval == 0); + // Add attributes to the changed set if they have been created in the last // iteration. ChangedAAs.append(AllAbstractAttributes.begin() + NumAAs, @@ -2589,13 +2611,18 @@ ChangeStatus Attributor::run() { size_t NumFinalAAs = AllAbstractAttributes.size(); + if (VerifyMaxFixpointIterations && IterationCounter != MaxFixpointIterations) { + errs() << "\n[Attributor] Fixpoint iteration done after: " + << IterationCounter << "/" << MaxFixpointIterations + << " iterations\n"; + llvm_unreachable("The fixpoint was not reached with exactly the number of " + "specified iterations!"); + } + LLVM_DEBUG(dbgs() << "\n[Attributor] Fixpoint iteration done after: " << IterationCounter << "/" << MaxFixpointIterations << " iterations\n"); - if (VerifyMaxFixpointIterations && IterationCounter != MaxFixpointIterations) - llvm_unreachable("The fixpoint was not reached with exactly the number of " - "specified iterations!"); bool FinishedAtFixpoint = Worklist.empty(); @@ -2930,7 +2957,7 @@ static bool runAttributorOnModule(Module &M) { // Create an Attributor and initially empty information cache that is filled // while we identify default attribute opportunities. InformationCache InfoCache(M.getDataLayout()); - Attributor A(InfoCache); + Attributor A(InfoCache, DepRecInterval); for (Function &F : M) { // TODO: Not all attributes require an exact definition. Find a way to diff --git a/test/Transforms/FunctionAttrs/align.ll b/test/Transforms/FunctionAttrs/align.ll index fd387911386..fd35b6964e0 100644 --- a/test/Transforms/FunctionAttrs/align.ll +++ b/test/Transforms/FunctionAttrs/align.ll @@ -1,4 +1,4 @@ -; RUN: opt -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=15 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR +; RUN: opt -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=14 -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" diff --git a/test/Transforms/FunctionAttrs/nonnull.ll b/test/Transforms/FunctionAttrs/nonnull.ll index e7fbb9194d3..b311c6f5587 100644 --- a/test/Transforms/FunctionAttrs/nonnull.ll +++ b/test/Transforms/FunctionAttrs/nonnull.ll @@ -1,6 +1,6 @@ ; RUN: opt -S -functionattrs -enable-nonnull-arg-prop %s | FileCheck %s --check-prefixes=BOTH,FNATTR ; RUN: opt -S -passes=function-attrs -enable-nonnull-arg-prop %s | FileCheck %s --check-prefixes=BOTH,FNATTR -; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=11 -S < %s | FileCheck %s --check-prefixes=BOTH,ATTRIBUTOR +; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=10 -S < %s | FileCheck %s --check-prefixes=BOTH,ATTRIBUTOR target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" diff --git a/test/Transforms/FunctionAttrs/noreturn_sync.ll b/test/Transforms/FunctionAttrs/noreturn_sync.ll index 615a7c4718e..4caf74cb967 100644 --- a/test/Transforms/FunctionAttrs/noreturn_sync.ll +++ b/test/Transforms/FunctionAttrs/noreturn_sync.ll @@ -1,4 +1,4 @@ -; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=7 -S < %s | FileCheck %s +; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=6 -S < %s | FileCheck %s ; ; This file is the same as noreturn_async.ll but with a personality which ; indicates that the exception handler *cannot* catch asynchronous exceptions. -- 2.50.1