Skip to content

[SILOptimizer] Added MandatoryCombiner. #26913

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
Aug 29, 2019
Merged
Show file tree
Hide file tree
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
4 changes: 3 additions & 1 deletion include/swift/SILOptimizer/PassManager/Passes.def
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
/// This macro follows the same conventions as PASS(Id, Tag, Description),
/// but is used for IRGen passes which are built outside of the
/// SILOptimizer library.
///
///
/// An IRGen pass is created by IRGen and needs to be registered with the pass
/// manager dynamically.
#ifndef IRGEN_PASS
Expand Down Expand Up @@ -312,6 +312,8 @@ PASS(SerializeSILPass, "serialize-sil",
PASS(YieldOnceCheck, "yield-once-check",
"Check correct usage of yields in yield-once coroutines")
PASS(OSLogOptimization, "os-log-optimization", "Optimize os log calls")
PASS(MandatoryCombiner, "mandatory-combiner",
"Perform mandatory peephole combines")
PASS(BugReducerTester, "bug-reducer-tester",
"sil-bug-reducer Tool Testing by Asserting on a Sentinel Function")
PASS_RANGE(AllPasses, AADumper, BugReducerTester)
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Mandatory/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@ silopt_register_sources(
SemanticARCOpts.cpp
SILGenCleanup.cpp
YieldOnceCheck.cpp
MandatoryCombiner.cpp
OSLogOptimization.cpp
)
174 changes: 174 additions & 0 deletions lib/SILOptimizer/Mandatory/MandatoryCombiner.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
//===------- MandatoryCombiner.cpp ----------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should establish the practice of adding file-level comments to new passes. What is the intention of the pass? Where is it expected and/or valid to run in the pipeline? Does it need to run before diagnostics? Or is it an optional Onone optimization? What are the pass dependencies--can other passes interfere with this, or is this helpful to run before certain passes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would something like this be?

///
///  \file
///  Defines the MandatoryCombiner function transform.  The pass contains basic
///  instruction combines to be performed at the begining of both the Onone and
///  also the performance pass pipelines, after the diagnostics passes have been
///  run.  It is intended to be run before and to be independent of other
///  transforms.
///
//===----------------------------------------------------------------------===//

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put a newline after \file.

Also can you put in some prose that says something along the lines of: "The intention of this pass is to be a place for mandatory peepholes that are not needed for diagnostics. Please put any such peepholes here instead of in the diagnostic passes".

///
/// \file
///
/// Defines the MandatoryCombiner function transform. The pass contains basic
/// instruction combines to be performed at the begining of both the Onone and
/// also the performance pass pipelines, after the diagnostics passes have been
/// run. It is intended to be run before and to be independent of other
/// transforms.
///
/// The intention of this pass is to be a place for mandatory peepholes that
/// are not needed for diagnostics. Please put any such peepholes here instead
/// of in the diagnostic passes.
///
//===----------------------------------------------------------------------===//

#define DEBUG_TYPE "sil-mandatory-combiner"
#include "swift/SIL/SILVisitor.h"
#include "swift/SILOptimizer/PassManager/Passes.h"
#include "swift/SILOptimizer/PassManager/Transforms.h"
#include "swift/SILOptimizer/Utils/Local.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <vector>

using namespace swift;

/// \returns whether all the values are of trivial type in the provided
/// function.
template <typename Values>
static bool areAllValuesTrivial(Values values, SILFunction &function) {
return llvm::all_of(values, [&](SILValue value) -> bool {
return value->getType().isTrivial(function);
});
}

/// Replaces all full applies of the specified partial apply with full applies
/// of the underlying function ref and attempts to delete the partial apply.
///
/// The transformation of the full applies only takes place if all the arguments
/// to the full applies are trivial.
///
/// TODO: Apply this transformation to partial applies not all of whose
/// arguments are trivial.
///
/// \param pai the partial apply to attempt to delete
///
/// \returns a pair of bools. The first bool indicates whether the a full apply
/// of the partial apply was deleted, meaning that analyses must be
/// invalidated. The second bool indicates that the provided partial
/// apply instruction itself was deleted, with consequences for the
/// iterator.
static std::pair<bool, bool> processPartialApply(PartialApplyInst *pai) {
auto callee = pai->getCallee();
LLVM_DEBUG(llvm::dbgs() << "Callee: " << *callee);

// Apply this pass only to partial applies all of whose arguments are
// trivial.
auto *function = pai->getReferencedFunctionOrNull();
if (!function) {
return {false, false};
}

auto paiArgs = ApplySite(pai).getArguments();
if (!areAllValuesTrivial(paiArgs, *function)) {
return {false, false};
}

bool erasedFullApply = false;
for (auto *use : pai->getUses()) {
auto fas = FullApplySite::isa(use->getUser());
if (!fas) {
continue;
}
LLVM_DEBUG(llvm::dbgs() << "Partial Apply: " << *pai);
LLVM_DEBUG(llvm::dbgs() << "Full Apply Site: " << *fas.getInstruction());
auto *fasi = dyn_cast<ApplyInst>(fas.getInstruction());
if (!fasi) {
continue;
}

auto fasArgs = fas.getArguments();
if (!areAllValuesTrivial(fasArgs, *function)) {
continue;
}

SmallVector<SILValue, 8> argsVec;
llvm::copy(paiArgs, std::back_inserter(argsVec));
llvm::copy(fasArgs, std::back_inserter(argsVec));

SILBuilderWithScope builder(fasi);
ApplyInst *appInst = builder.createApply(
/*Loc=*/fasi->getDebugLocation().getLocation(), /*Fn=*/callee,
/*Subs=*/pai->getSubstitutionMap(),
/*Args*/ argsVec,
/*isNonThrowing=*/fasi->isNonThrowing(),
/*SpecializationInfo=*/pai->getSpecializationInfo());
fasi->replaceAllUsesWith(appInst);
fasi->eraseFromParent();
erasedFullApply = true;
}
bool deletedDeadClosure = tryDeleteDeadClosure(pai);
return {erasedFullApply, deletedDeadClosure};
}

//===----------------------------------------------------------------------===//
// Top Level Entrypoint
//===----------------------------------------------------------------------===//

namespace {

struct MandatoryCombiner : SILFunctionTransform {
void run() override {
bool madeChange = false;
auto *f = getFunction();
for (auto &block : *f) {
for (auto ii = block.begin(), ie = block.end(); ii != ie;) {
auto *inst = &*ii;
// If any action is taken, the current instruction will be deleted.
// That instruction would be the definition of a partial apply. Because
// the definition dominates all its uses, the previous instruction will
// be unaffected by the removal of the instruction and its uses. So
// move the iterator back a step. If no action is taken, it will be
// advanced twice. If an action is taken, it will be advanced once to a
// new instruction (since the current one will have been deleted).
ii = prev_or_default(ii, block.begin(), block.end());

bool deleted = false;
if (auto *pai = dyn_cast<PartialApplyInst>(inst)) {
auto result = processPartialApply(pai);
deleted = result.second;
madeChange |= result.first || result.second;
}

if (deleted) {
// The deletion succeeded. The iterator has already been moved back
// a step so as to avoid invalidation. Advancing it one step will
// not advance it to the current instruction since that has been
// deleted. Instead, it will advance it to the next *remaining*
// instruction after the previous instruction as desired.
ii = next_or_default(ii, /*end*/block.end(), /*defaultIter*/block.begin());
continue;
}

// No action was taken. The iterator has already been moved back a
// step. Consequently it is insufficient to advance it once--it must
// be advanced twice.
ii = next_or_default(ii, /*end*/block.end(), /*defaultIter*/block.begin());
++ii;
}
}

if (madeChange) {
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
}
}
};

} // end anonymous namespace

SILTransform *swift::createMandatoryCombiner() {
return new MandatoryCombiner();
}
Loading