-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[mandatory-combine] Add a canonicalization impl and prepare to use mandatory-combine to test ownership RAUW code #34968
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
8f1b353
04a1a72
c6525fd
28e7e95
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 |
---|---|---|
|
@@ -31,6 +31,7 @@ | |
#include "swift/SIL/SILVisitor.h" | ||
#include "swift/SILOptimizer/PassManager/Passes.h" | ||
#include "swift/SILOptimizer/PassManager/Transforms.h" | ||
#include "swift/SILOptimizer/Utils/CanonicalizeInstruction.h" | ||
#include "swift/SILOptimizer/Utils/InstOptUtils.h" | ||
#include "swift/SILOptimizer/Utils/StackNesting.h" | ||
#include "llvm/ADT/STLExtras.h" | ||
|
@@ -53,6 +54,54 @@ static bool areAllValuesTrivial(Values values, SILFunction &function) { | |
}); | ||
} | ||
|
||
//===----------------------------------------------------------------------===// | ||
// CanonicalizeInstruction subclass for use in Mandatory Combiner. | ||
//===----------------------------------------------------------------------===// | ||
|
||
namespace { | ||
|
||
class MandatoryCombineCanonicalize final : CanonicalizeInstruction { | ||
public: | ||
using Worklist = SmallSILInstructionWorklist<256>; | ||
|
||
private: | ||
Worklist &worklist; | ||
bool changed = false; | ||
|
||
public: | ||
MandatoryCombineCanonicalize(Worklist &worklist) | ||
: CanonicalizeInstruction(DEBUG_TYPE), worklist(worklist) {} | ||
|
||
void notifyNewInstruction(SILInstruction *inst) override { | ||
worklist.add(inst); | ||
worklist.addUsersOfAllResultsToWorklist(inst); | ||
changed = true; | ||
} | ||
|
||
// Just delete the given 'inst' and record its operands. The callback isn't | ||
// allowed to mutate any other instructions. | ||
void killInstruction(SILInstruction *inst) override { | ||
worklist.eraseSingleInstFromFunction(*inst, | ||
/*AddOperandsToWorklist*/ true); | ||
changed = true; | ||
} | ||
|
||
void notifyHasNewUsers(SILValue value) override { | ||
if (worklist.size() < 10000) { | ||
worklist.addUsersToWorklist(value); | ||
} | ||
changed = true; | ||
} | ||
|
||
bool tryCanonicalize(SILInstruction *inst) { | ||
changed = false; | ||
canonicalize(inst); | ||
return changed; | ||
} | ||
}; | ||
|
||
} // anonymous namespace | ||
|
||
//===----------------------------------------------------------------------===// | ||
// MandatoryCombiner Interface | ||
//===----------------------------------------------------------------------===// | ||
|
@@ -137,6 +186,13 @@ class MandatoryCombiner final | |
// MandatoryCombiner Non-Visitor Utility Methods | ||
//===----------------------------------------------------------------------===// | ||
|
||
static llvm::cl::opt<bool> EnableCanonicalizationAndTrivialDCE( | ||
"sil-mandatory-combine-enable-canon-and-simple-dce", llvm::cl::Hidden, | ||
llvm::cl::init(false), | ||
llvm::cl::desc("An option for compiler developers that cause the Mandatory " | ||
"Combiner to be more aggressive at eliminating trivially " | ||
"dead code and canonicalizing SIL")); | ||
|
||
void MandatoryCombiner::addReachableCodeToWorklist(SILFunction &function) { | ||
SmallVector<SILBasicBlock *, 32> blockWorklist; | ||
SmallPtrSet<SILBasicBlock *, 32> blockAlreadyAddedToWorklist; | ||
|
@@ -148,6 +204,8 @@ void MandatoryCombiner::addReachableCodeToWorklist(SILFunction &function) { | |
blockAlreadyAddedToWorklist.insert(firstBlock); | ||
} | ||
|
||
bool compilingWithOptimization = function.getEffectiveOptimizationMode() != | ||
OptimizationMode::NoOptimization; | ||
while (!blockWorklist.empty()) { | ||
auto *block = blockWorklist.pop_back_val(); | ||
|
||
|
@@ -156,6 +214,12 @@ void MandatoryCombiner::addReachableCodeToWorklist(SILFunction &function) { | |
++iterator; | ||
|
||
if (isInstructionTriviallyDead(instruction)) { | ||
if (EnableCanonicalizationAndTrivialDCE) { | ||
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. Could these be flattened into one 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. This is something that is meant to be deleted soon. I think it is fine given that this is temporary. I need this for testing purposes for something else. |
||
if (compilingWithOptimization) { | ||
instruction->replaceAllUsesOfAllResultsWithUndef(); | ||
instruction->eraseFromParent(); | ||
} | ||
} | ||
continue; | ||
} | ||
|
||
|
@@ -177,13 +241,32 @@ bool MandatoryCombiner::doOneIteration(SILFunction &function, | |
madeChange = false; | ||
|
||
addReachableCodeToWorklist(function); | ||
MandatoryCombineCanonicalize mcCanonicialize(worklist); | ||
|
||
bool compilingWithOptimization = function.getEffectiveOptimizationMode() != | ||
OptimizationMode::NoOptimization; | ||
|
||
while (!worklist.isEmpty()) { | ||
auto *instruction = worklist.pop_back_val(); | ||
if (instruction == nullptr) { | ||
continue; | ||
} | ||
|
||
if (EnableCanonicalizationAndTrivialDCE) { | ||
if (compilingWithOptimization) { | ||
if (isInstructionTriviallyDead(instruction)) { | ||
worklist.eraseInstFromFunction(*instruction); | ||
madeChange = true; | ||
continue; | ||
} | ||
} | ||
|
||
if (mcCanonicialize.tryCanonicalize(instruction)) { | ||
madeChange = true; | ||
continue; | ||
} | ||
} | ||
|
||
#ifndef NDEBUG | ||
std::string instructionDescription; | ||
#endif | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,12 @@ sil @myint_from_myint_and_proto : $@convention(thin) (MyInt, @guaranteed { var P | |
|
||
sil @myint_from_proto_and_myint : $@convention(thin) (@guaranteed { var Proto }, MyInt) -> MyInt | ||
|
||
// Optional support | ||
enum FakeOptional<T> { | ||
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. Looks like this declaration isn't actually used in the test...? It would be good to test a Swift program that uses this type, though. That's where we've run into problems with this optimization in the past. 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. This is just misc boiler plate tests that I add in most tests. Its useful to have. |
||
case none | ||
case some(T) | ||
} | ||
|
||
/////////// | ||
// Tests // | ||
/////////// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
// RUN: %target-sil-opt -mandatory-combine -sil-mandatory-combine-enable-canon-and-simple-dce %s | %FileCheck %s | ||
|
||
// Tests for when the mandatory combiner is running with optimizations | ||
// enabled. Only put tests here for functionality that only occurs when the | ||
// Mandatory Combiner runs in between the diagnostics/perf pipeline at -O, | ||
// -Osize. | ||
|
||
sil_stage canonical | ||
|
||
import Builtin | ||
|
||
// Trivial declarations | ||
|
||
struct MyInt { | ||
var value: Builtin.Int64 | ||
} | ||
|
||
// Generic declarations | ||
|
||
protocol Addable { | ||
static var an: Self { get } | ||
} | ||
|
||
// Class declarations | ||
|
||
class Klass { | ||
init() | ||
deinit | ||
} | ||
|
||
// Existential declarations | ||
|
||
protocol Proto { | ||
static var an: Proto { get } | ||
} | ||
|
||
// Trivial support | ||
|
||
sil @first_of_three_ints : $@convention(thin) (MyInt, MyInt, MyInt) -> MyInt | ||
|
||
sil @constant_zero : $@convention(thin) () -> MyInt | ||
|
||
sil @identity_int : $@convention(thin) (MyInt) -> MyInt | ||
|
||
// Generic support | ||
|
||
sil @first_of_three_addables : $@convention(thin) <A where A : Addable> (@in_guaranteed A, @guaranteed <τ_0_0 where τ_0_0 : Addable> { var τ_0_0 } <A>, @guaranteed <τ_0_0 where τ_0_0 : Addable> { var τ_0_0 } <A>) -> @ | ||
out A | ||
|
||
// Class support | ||
|
||
sil [exact_self_class] @klass_alloc_init : $@convention(method) (@thick Klass.Type) -> @owned Klass | ||
|
||
// Klass.init() | ||
sil @klass_init : $@convention(method) (@owned Klass) -> @owned Klass | ||
// Klass.deinit | ||
sil @klass_deinit : $@convention(method) (@guaranteed Klass) -> @owned Builtin.NativeObject | ||
|
||
// Klass.__deallocating_deinit | ||
sil @klass_dealloc_deinit : $@convention(method) (@owned Klass) -> () | ||
|
||
sil_vtable Klass { | ||
#Klass.init!allocator: (Klass.Type) -> () -> Klass : @klass_alloc_init | ||
#Klass.deinit!deallocator: @klass_dealloc_deinit | ||
} | ||
|
||
sil @first_of_three_klasses : $@convention(thin) (@guaranteed Klass, @guaranteed Klass, @guaranteed Klass) -> @owned Klass | ||
|
||
// Existential support | ||
|
||
sil @first_of_three_protos : $@convention(thin) (@in_guaranteed Proto, @guaranteed { var Proto }, @guaranteed { var Proto }) -> @out Proto | ||
|
||
sil @get_proto : $@convention(thin) () -> @out Proto | ||
|
||
// Mixed support | ||
|
||
sil @proto_from_proto_and_myint : $@convention(thin) (@in_guaranteed Proto, MyInt) -> @out Proto | ||
|
||
sil @myint_from_myint_and_proto : $@convention(thin) (MyInt, @guaranteed { var Proto }) -> MyInt | ||
|
||
sil @myint_from_proto_and_myint : $@convention(thin) (@guaranteed { var Proto }, MyInt) -> MyInt | ||
|
||
// Optional support | ||
enum FakeOptional<T> { | ||
case none | ||
case some(T) | ||
} | ||
|
||
/////////// | ||
// Tests // | ||
/////////// | ||
|
||
|
||
// CHECK-LABEL: sil [ossa] @eliminate_simple_arc_traffic : $@convention(thin) (@guaranteed Klass) -> () { | ||
// CHECK-NOT: copy_value | ||
// CHECK-NOT: destroy_value | ||
// CHECK-NOT: enum | ||
// CHECK-NOT: end_borrow | ||
// CHECK: } // end sil function 'eliminate_simple_arc_traffic' | ||
sil [ossa] @eliminate_simple_arc_traffic : $@convention(thin) (@guaranteed Klass) -> () { | ||
bb0(%0 : @guaranteed $Klass): | ||
%1 = copy_value %0 : $Klass | ||
destroy_value %1 : $Klass | ||
%2 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt | ||
end_borrow %2 : $FakeOptional<Klass> | ||
%9999 = tuple() | ||
return %9999 : $() | ||
} |
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.
We're already "doing the work" in Onone. We should either just erase the known-dead instruction, or not call
isInstructionTriviallyDead
at all (preferably the later).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.
We need to know to skip it. We are keeping the size of the worklist down.
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.
Fair enough.