-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[InlineCost] Cache collectEphemeralValues() to save compile time #130210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
//===- llvm/Analysis/EphemeralValuesCache.h ---------------------*- C++ -*-===// | ||
// | ||
// 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 | ||
// | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This pass caches ephemeral values, i.e., values that are only used by | ||
// @llvm.assume intrinsics, for cheap access after the initial collection. | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#ifndef LLVM_ANALYSIS_EPHEMERALVALUESCACHE_H | ||
#define LLVM_ANALYSIS_EPHEMERALVALUESCACHE_H | ||
|
||
#include "llvm/ADT/SmallPtrSet.h" | ||
#include "llvm/IR/PassManager.h" | ||
|
||
namespace llvm { | ||
|
||
class Function; | ||
class AssumptionCache; | ||
class Value; | ||
|
||
/// A cache of ephemeral values within a function. | ||
class EphemeralValuesCache { | ||
SmallPtrSet<const Value *, 32> EphValues; | ||
Function &F; | ||
AssumptionCache &AC; | ||
bool Collected = false; | ||
|
||
void collectEphemeralValues(); | ||
|
||
public: | ||
EphemeralValuesCache(Function &F, AssumptionCache &AC) : F(F), AC(AC) {} | ||
void clear() { | ||
EphValues.clear(); | ||
Collected = false; | ||
} | ||
const SmallPtrSetImpl<const Value *> &ephValues() { | ||
if (!Collected) | ||
collectEphemeralValues(); | ||
return EphValues; | ||
} | ||
}; | ||
|
||
class EphemeralValuesAnalysis | ||
: public AnalysisInfoMixin<EphemeralValuesAnalysis> { | ||
friend AnalysisInfoMixin<EphemeralValuesAnalysis>; | ||
static AnalysisKey Key; | ||
|
||
public: | ||
using Result = EphemeralValuesCache; | ||
Result run(Function &F, FunctionAnalysisManager &FAM); | ||
}; | ||
|
||
} // namespace llvm | ||
|
||
#endif // LLVM_ANALYSIS_EPHEMERALVALUESCACHE_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
//===- EphemeralValuesCache.cpp - Cache collecting ephemeral values -------===// | ||
// | ||
// 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/EphemeralValuesCache.h" | ||
#include "llvm/Analysis/AssumptionCache.h" | ||
#include "llvm/Analysis/CodeMetrics.h" | ||
|
||
namespace llvm { | ||
|
||
void EphemeralValuesCache::collectEphemeralValues() { | ||
CodeMetrics::collectEphemeralValues(&F, &AC, EphValues); | ||
Collected = true; | ||
} | ||
|
||
AnalysisKey EphemeralValuesAnalysis::Key; | ||
|
||
EphemeralValuesCache | ||
EphemeralValuesAnalysis::run(Function &F, FunctionAnalysisManager &FAM) { | ||
auto &AC = FAM.getResult<AssumptionAnalysis>(F); | ||
return EphemeralValuesCache(F, AC); | ||
} | ||
|
||
} // namespace llvm |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ | |
#include "llvm/Analysis/BasicAliasAnalysis.h" | ||
#include "llvm/Analysis/BlockFrequencyInfo.h" | ||
#include "llvm/Analysis/CGSCCPassManager.h" | ||
#include "llvm/Analysis/EphemeralValuesCache.h" | ||
#include "llvm/Analysis/InlineAdvisor.h" | ||
#include "llvm/Analysis/InlineCost.h" | ||
#include "llvm/Analysis/LazyCallGraph.h" | ||
|
@@ -388,6 +389,9 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC, | |
Advice->recordUnsuccessfulInlining(IR); | ||
continue; | ||
} | ||
// TODO: Shouldn't we be invalidating all analyses on F here? | ||
// The caller was modified, so invalidate Ephemeral Values. | ||
FAM.getResult<EphemeralValuesAnalysis>(F).clear(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, shouldn't we be invalidating all analyses on the function here, rather than just EphemeralValues? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so, but let's make this a separate patch in case it breaks something. I added a TODO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, that sounds reasonable. |
||
|
||
DidInline = true; | ||
InlinedCallees.insert(&Callee); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need an extra layer of indirection? can't we just eagerly calculate the ephemeral values in
run()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean removing the indirection by incorporating the functionality of the
EphemeralValuesCache
inside theEphemeralValuesAnalysis
class? Isn't it better to follow the two-class design where you implement the analysis in one class and have a separate wrapper class for the pass in case someone needs to use the analysis without a pass ?As for the eager vs lazy approach, my guess is that lazy is preferable because it may save you some compile time if you don't end up calling the analysis, but I don't feel strongly about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeMetrics::collectEphemeralValues()
is the API if someone wants to calculate ephemeral values, I don't think we need another layer on top of that. I thinkEphemeralValuesAnalysis::Result
could just beSmallPtrSet<const Value *, 32>
the pass should only call
FAM.getAnalysis
if it's going to use it, so the laziness should be done at the caller level