Skip to content

[globalopt] Clean up doxygen comments in GlobalOpt.cpp. #15480

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

Merged
merged 1 commit into from
Mar 24, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 60 additions & 30 deletions lib/SILOptimizer/IPO/GlobalOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,33 +51,47 @@ class SILGlobalOpt {
DominanceAnalysis* DA;
bool HasChanged = false;

// Map each global initializer to a list of call sites.
typedef SmallVector<ApplyInst *, 4> GlobalInitCalls;
typedef SmallVector<LoadInst *, 4> GlobalLoads;

/// A map from each visited global initializer call to a list of call sites.
llvm::MapVector<SILFunction *, GlobalInitCalls> GlobalInitCallMap;

// The following mappings are used if this is a compilation
// in scripting mode and global variables are accessed without
// addressors.

// Map each global let variable to a set of loads from it.
/// A map from each visited global let variable to its set of loads.
llvm::MapVector<SILGlobalVariable *, GlobalLoads> GlobalLoadMap;
// Map each global let variable to the store instruction which initializes it.

/// A map from each visited global let variable to the store instructions
/// which initialize it.
llvm::MapVector<SILGlobalVariable *, StoreInst *> GlobalVarStore;
// Variables in this set should not be processed by this pass
// anymore.

/// A set of visited global variables that for some reason we have decided is
/// not able to be optimized safely or for which we do not know how to
/// optimize safely.
///
/// Once a global variable is in this set, we no longer will process it.
llvm::SmallPtrSet<SILGlobalVariable *, 16> GlobalVarSkipProcessing;

// Mark any block that this pass has determined to be inside a loop.
/// The set of blocks that this pass has determined to be inside a loop.
///
/// This is used to mark any block that this pass has determined to be inside
/// a loop.
llvm::DenseSet<SILBasicBlock *> LoopBlocks;
// Mark any functions for which loops have been analyzed.

/// The set of functions that have had their loops analyzed.
llvm::DenseSet<SILFunction *> LoopCheckedFunctions;
// Keep track of cold blocks.

/// Keep track of cold blocks.
ColdBlockInfo ColdBlocks;

// Whether we see a "once" call to callees that we currently don't handle.
/// Whether we see a "once" call to callees that we currently don't handle.
bool UnhandledOnceCallee = false;
// Record number of times a globalinit_func is called by "once".

/// A map from a globalinit_func to the number of times "once" has called the
/// function.
llvm::DenseMap<SILFunction *, unsigned> InitializerCount;
public:
SILGlobalOpt(SILModule *M, DominanceAnalysis *DA)
Expand All @@ -86,23 +100,40 @@ class SILGlobalOpt {
bool run();

protected:
/// If this is a call to a global initializer, map it.
void collectGlobalInitCall(ApplyInst *AI);

/// If this load is a read from a global let variable, add the load to
/// GlobalLoadMap[SILG].
void collectGlobalLoad(LoadInst *SI, SILGlobalVariable *SILG);

/// If this store is a write to a global let variable, add the store to
/// GlobalStoreMap[SILG].
void collectGlobalStore(StoreInst *SI, SILGlobalVariable *SILG);

/// This is the main entrypoint for collecting global accesses.
void collectGlobalAccess(GlobalAddrInst *GAI);

SILGlobalVariable *getVariableOfGlobalInit(SILFunction *AddrF);

/// Returns true if we think that \p CurBB is inside a loop.
bool isInLoop(SILBasicBlock *CurBB);
void placeInitializers(SILFunction *InitF, ArrayRef<ApplyInst *> Calls);

// Update UnhandledOnceCallee and InitializerCount by going through all "once"
// calls.
/// Update UnhandledOnceCallee and InitializerCount by going through all
/// "once" calls.
void collectOnceCall(BuiltinInst *AI);
// Set the static initializer and remove "once" from addressor if a global can
// be statically initialized.

/// Set the static initializer and remove "once" from addressor if a global
/// can be statically initialized.
void optimizeInitializer(SILFunction *AddrF, GlobalInitCalls &Calls);

/// Optimize access to the global variable, which is known to have a constant
/// value. Replace all loads from the global address by invocations of a
/// getter that returns the value of this variable.
void optimizeGlobalAccess(SILGlobalVariable *SILG, StoreInst *SI);
// Replace loads from a global variable by the known value.

/// Replace loads from a global variable by the known value.
void replaceLoadsByKnownValue(BuiltinInst *CallToOnce,
SILFunction *AddrF,
SILFunction *InitF,
Expand All @@ -119,11 +150,11 @@ class InstructionsCloner : public SILClonerWithScopes<InstructionsCloner> {

ArrayRef<SILInstruction *> Insns;

protected:
protected:
SILBasicBlock *FromBB, *DestBB;

public:
// A map of old to new available values.
public:
/// A map of old to new available values.
SmallVector<std::pair<ValueBase *, SILValue>, 16> AvailVals;

InstructionsCloner(SILFunction &F,
Expand All @@ -148,7 +179,7 @@ class InstructionsCloner : public SILClonerWithScopes<InstructionsCloner> {
AvailVals.push_back(std::make_pair(origResults[i], clonedResults[i]));
}

// Clone all instructions from Insns into DestBB
/// Clone all instructions from Insns into DestBB
void clone() {
for (auto I : Insns)
process(I);
Expand All @@ -157,7 +188,7 @@ class InstructionsCloner : public SILClonerWithScopes<InstructionsCloner> {

} // end anonymous namespace

/// If this is a call to a global initializer, map it.
// If this is a call to a global initializer, map it.
void SILGlobalOpt::collectGlobalInitCall(ApplyInst *AI) {
SILFunction *F = AI->getReferencedFunction();
if (!F || !F->isGlobalInit())
Expand All @@ -166,7 +197,7 @@ void SILGlobalOpt::collectGlobalInitCall(ApplyInst *AI) {
GlobalInitCallMap[F].push_back(AI);
}

/// If this is a read from a global let variable, map it.
// If this is a read from a global let variable, map it.
void SILGlobalOpt::collectGlobalLoad(LoadInst *LI, SILGlobalVariable *SILG) {
assert(SILG);
//assert(SILG->isLet());
Expand Down Expand Up @@ -238,8 +269,8 @@ static SILFunction *getGlobalGetterFunction(SILModule &M,
IsBare, IsNotTransparent, Serialized);
}

/// Generate getter from the initialization code whose
/// result is stored by a given store instruction.
/// Generate getter from the initialization code whose result is stored by a
/// given store instruction.
static SILFunction *genGetterFromInit(StoreInst *Store,
SILGlobalVariable *SILG) {
auto *varDecl = SILG->getDecl();
Expand Down Expand Up @@ -300,7 +331,7 @@ static SILFunction *genGetterFromInit(StoreInst *Store,
return GetterF;
}

/// If this is a read from a global let variable, map it.
// If this is a write to a global let variable, map it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some guiding principle for when you're using doc-comments vs. normal comments?

(I personally find the whole thing silly since no one would ever be reading a doc page to understand the internal implementation of a SIL pass, but if I know the rule I'll try to follow it)

Copy link
Contributor Author

@gottesmm gottesmm Mar 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes there is a principle. In the body of a method/function always '//'. On a method declaration, '///' this is a doxygen comment that should show up. On a method impl, you should add file level comments as '//'. These are intended to not be in the public interface, but are more implementation details. Functions that are static and are not declared anywhere separately, I will use three slashes since that is the interface to the function.

void SILGlobalOpt::collectGlobalStore(StoreInst *SI, SILGlobalVariable *SILG) {

if (GlobalVarStore.count(SILG)) {
Expand Down Expand Up @@ -328,8 +359,8 @@ static SILFunction *getCalleeOfOnceCall(BuiltinInst *BI) {
return nullptr;
}

/// Update UnhandledOnceCallee and InitializerCount by going through all "once"
/// calls.
// Update UnhandledOnceCallee and InitializerCount by going through all "once"
// calls.
void SILGlobalOpt::collectOnceCall(BuiltinInst *BI) {
if (UnhandledOnceCallee)
return;
Expand Down Expand Up @@ -902,10 +933,9 @@ void SILGlobalOpt::collectGlobalAccess(GlobalAddrInst *GAI) {
}
}

/// Optimize access to the global variable, which is known
/// to have a constant value. Replace all loads from the
/// global address by invocations of a getter that returns
/// the value of this variable.
// Optimize access to the global variable, which is known to have a constant
// value. Replace all loads from the global address by invocations of a getter
// that returns the value of this variable.
void SILGlobalOpt::optimizeGlobalAccess(SILGlobalVariable *SILG,
StoreInst *SI) {
DEBUG(llvm::dbgs() << "GlobalOpt: use static initializer for " <<
Expand Down