Skip to content
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

Extract recording #1245

Merged
merged 56 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
a1a4129
Extracted callees
fikovnik Jun 20, 2023
0639778
Removing the callees from more places
fikovnik Jun 21, 2023
1a3c571
Keep the information at the Function level
fikovnik Jun 26, 2023
55abdeb
Added support for test and type
fikovnik Jun 26, 2023
a8dd21a
Missed a place to update the feedback
fikovnik Jul 7, 2023
0d3819d
Fix type feedback from closures
fikovnik Jul 11, 2023
ca8fd9c
Consolidate names - use Type instead of Value
fikovnik Jul 11, 2023
c524a5d
Fixed typo
fikovnik Jul 11, 2023
f4670e8
Fix missing dispatch table
fikovnik Jul 12, 2023
e6f84ee
Add target into the record instruction
fikovnik Jul 13, 2023
7c3023e
Update the print methods on DT
fikovnik Jul 13, 2023
a24e34a
Remove the support for Record in PIR
fikovnik Jul 13, 2023
6829dc9
Cleanup
fikovnik Jul 13, 2023
c1d1acd
Another cleanup
fikovnik Jul 13, 2023
07b5fd3
Add support for TF serialization and deserialization
fikovnik Jul 14, 2023
cc832d8
Fixed missed initialization of TypeFeedback::owner_.
fikovnik Jul 14, 2023
b9c99de
Fixes
fikovnik Jul 14, 2023
1be92d9
Fixes - attempt to find the leak
fikovnik Jul 18, 2023
7acb4e8
Fixup
fikovnik Jul 19, 2023
a23369e
Fix deserialization
fikovnik Jul 19, 2023
799bb95
Refactor TypeFeedback to be RirRuntimeObject
fikovnik Jul 20, 2023
69e6254
Add ninja command allowing to rebuild without leaving gdb
fikovnik Jul 20, 2023
e5748ef
Fix TypeFeedback constructor when slots ar empty
fikovnik Jul 20, 2023
14c3fb7
Inline type feedback info into BC instructions
fikovnik Jul 24, 2023
02207c0
Parameterize the BC code size for OSR triggering
fikovnik Jul 25, 2023
a7791dc
Fix invalid type feedback offset
fikovnik Jul 25, 2023
353d69a
Make the code as it was in master
fikovnik Jul 25, 2023
9667bba
Fix type feedback slot indices
fikovnik Aug 7, 2023
490aaef
Cache typeFeedback pointer in interpreter
fikovnik Aug 7, 2023
06bd7d6
Pin the ReBench version to v1.1.0 which works
fikovnik Aug 9, 2023
1d9fbd3
Use three arrays instead of one for the type feedback
fikovnik Aug 15, 2023
4928ba2
Fix missed typed feedback from OSR
fikovnik Aug 15, 2023
b1466ad
Address cppcheck issues
fikovnik Aug 15, 2023
9e95ce7
Fix printing typefeedback without slot
fikovnik Aug 17, 2023
9290619
Remove fixed FIXMEs
fikovnik Aug 18, 2023
17a1227
Merge branch 'master' into extract-recording
fikovnik Aug 28, 2023
6c35787
Simplify to use just references instead of moves
fikovnik Aug 29, 2023
16a2086
Test pipeline 1
fikovnik Aug 31, 2023
dde6739
Disabling time-based compilation heuristics
fikovnik Sep 20, 2023
c044f13
Do not reoptimize already optimized functions
fikovnik Sep 20, 2023
744a697
Remove time-based heuristics and add type feedback versioning
fikovnik Sep 29, 2023
50b8d80
Fix serialization
fikovnik Sep 29, 2023
82ec9fc
Fix missing protect
fikovnik Sep 29, 2023
768810d
Lowering PIR_WARMUP to 50
fikovnik Oct 2, 2023
7818b6b
Set PIR_WARMUP just as a test
fikovnik Oct 6, 2023
f2c1866
Playing with tresholds
fikovnik Oct 13, 2023
c08f12f
Merge branch 'master' into extract-recording
fikovnik Oct 17, 2023
f80de27
Merge branch 'master' into extract-recording
fikovnik Oct 23, 2023
0d9889a
Disable pir_check to run in deopt chaos (fixes #1258)
fikovnik Nov 7, 2023
8abba87
Fix null deref in the case of failed OSR
fikovnik Nov 8, 2023
08c4144
Set PIR_WARMUP to 3
fikovnik Nov 8, 2023
1a3bcdf
Set PIR_WARMUP to 100
fikovnik Nov 8, 2023
78c4538
Cleanup
fikovnik Nov 8, 2023
4998a9b
Applying Seb's comments
fikovnik Nov 21, 2023
2642012
Pull back time based heurictic
fikovnik Nov 22, 2023
f8de583
Reset thresolds back to master
fikovnik Nov 22, 2023
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
Prev Previous commit
Next Next commit
Add target into the record instruction
  • Loading branch information
fikovnik committed Jul 13, 2023
commit e6f84ee12852237578e94c780848ecbc4f70af9b
32 changes: 10 additions & 22 deletions rir/src/compiler/native/builtins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -957,42 +957,30 @@ void deoptImpl(rir::Code* c, SEXP cls, DeoptMetadata* m, R_bcstack_t* args,
assert(false);
}

void recordTypefeedbackImpl(rir::Code* code, uint32_t idx, SEXP value) {
void recordTypefeedbackImpl(rir::TypeFeedback* typeFeedback, uint32_t idx,
SEXP value) {
// we cannot pass the feedback directly because the call to this builtin is
// generated from places that do not have access to the feedback vector
auto dt = code->function()->dispatchTable();
auto baseline = dt->baseline();
auto& slot = baseline->typeFeedback()[idx];

switch (slot.kind) {
case TypeFeedbackKind::Call: {
auto& feedback = slot.callees();
feedback.record(baseline->body(), value);
break;
}
case TypeFeedbackKind::Test: {
auto& feedback = slot.test();
feedback.record(value);
break;
}
case TypeFeedbackKind::Type: {
typeFeedback->record(idx, value);

auto& slot = (*typeFeedback)[idx];
if (slot.kind == TypeFeedbackKind::Type) {
auto& feedback = slot.type();
feedback.record(value);

if (TYPEOF(value) == PROMSXP) {
if (PRVALUE(value) == R_UnboundValue &&
feedback.stateBeforeLastForce < ObservedValues::promise)
feedback.stateBeforeLastForce < ObservedValues::promise) {
feedback.stateBeforeLastForce = ObservedValues::promise;
else if (feedback.stateBeforeLastForce <
ObservedValues::evaluatedPromise)
} else if (feedback.stateBeforeLastForce <
ObservedValues::evaluatedPromise) {
feedback.stateBeforeLastForce =
ObservedValues::evaluatedPromise;
}
} else {
if (feedback.stateBeforeLastForce < ObservedValues::value)
feedback.stateBeforeLastForce = ObservedValues::value;
}
}
}
}

void assertFailImpl(const char* msg) {
Expand Down
22 changes: 16 additions & 6 deletions rir/src/compiler/native/lower_function_llvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3602,7 +3602,8 @@ void LowerFunctionLLVM::compile() {

call(
NativeBuiltins::get(NativeBuiltins::Id::recordTypefeedback),
{paramCode(), c(rec->idx), loadSxp(rec->arg(0).val())});
{convertToPointer(rec->feedback, t::i8, true), c(rec->idx),
loadSxp(rec->arg(0).val())});

break;
}
Expand Down Expand Up @@ -6132,19 +6133,28 @@ void LowerFunctionLLVM::compile() {

// For OSR-in try to collect more typefeedback for the part of the
// code that was not yet executed.
// FIXME: is this correct? the feedbackOrigin index?
if (cls->isContinuation() && Rep::Of(i) == Rep::SEXP &&
variables_.count(i) &&
!cls->isContinuation()->continuationContext->asDeoptContext()) {
if (i->hasTypeFeedback()) {
call(NativeBuiltins::get(
NativeBuiltins::Id::recordTypefeedback),
{paramCode(),
c(i->typeFeedback().feedbackOrigin.idx()), load(i)});
auto& origin = i->typeFeedback().feedbackOrigin;
if (origin.isValid()) {
call(NativeBuiltins::get(
NativeBuiltins::Id::recordTypefeedback),
{convertToPointer(
&origin.function()->typeFeedback(), t::i8,
true),
c(origin.idx()), load(i)});
}
}
if (i->hasCallFeedback()) {
call(NativeBuiltins::get(
NativeBuiltins::Id::recordTypefeedback),
{paramCode(),
{convertToPointer(&i->callFeedback()
.feedbackOrigin.function()
->typeFeedback(),
t::i8, true),
c(i->typeFeedback().feedbackOrigin.idx()), load(i)});
}
}
Expand Down
2 changes: 2 additions & 0 deletions rir/src/compiler/pir/builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ Builder::Builder(Continuation* cnt, Value* closureEnv)
}
auto mkenv = new MkEnv(closureEnv, names, args.data(), miss);

// FIXME: what does this mean, we need both rirFun and we need idx
fikovnik marked this conversation as resolved.
Show resolved Hide resolved
mkenv->updateTypeFeedback().feedbackOrigin.function(
cnt->owner()->rirFunction());
add(mkenv);
Expand Down Expand Up @@ -173,6 +174,7 @@ Builder::Builder(ClosureVersion* version, Value* closureEnv)
auto rirFun = version->owner()->rirFunction();
if (rirFun->flags.contains(rir::Function::NeedsFullEnv))
mkenv->neverStub = true;
// FIXME: what does this mean, we need both rirFun and we need idx
mkenv->updateTypeFeedback().feedbackOrigin.function(rirFun);
add(mkenv);
this->env = mkenv;
Expand Down
7 changes: 4 additions & 3 deletions rir/src/compiler/pir/instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1019,10 +1019,11 @@ bool Deopt::hasDeoptReason() const {
return deoptReason() != DeoptReasonWrapper::unknown();
}

Record::Record(rir::TypeFeedbackKind kind, uint32_t idx)
Record::Record(rir::TypeFeedback* feedback, rir::TypeFeedbackKind kind,
uint32_t idx)
: FixedLenInstruction(PirType::voyd(), {{PirType::any()}},
{{UnknownDeoptTrigger::instance()}}),
kind(kind), idx(idx) {}
feedback(feedback), kind(kind), idx(idx) {}

Value* Record::getValue() const { return arg<0>().val(); }
void Record::setValue(Value* value) { arg<0>().val() = value; }
Expand All @@ -1040,7 +1041,7 @@ void Record::printArgs(std::ostream& out, bool tty) const {
break;
}

out << "#" << idx << " ";
out << "#" << idx << " (" << feedback << ") ";

getValue()->printRef(out);
}
Expand Down
4 changes: 3 additions & 1 deletion rir/src/compiler/pir/instruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -2720,10 +2720,12 @@ class Record
: public FixedLenInstruction<Tag::Record, Record, 1, Effects::AnyI(),
HasEnvSlot::No, Controlflow::None> {
public:
rir::TypeFeedback* feedback;
rir::TypeFeedbackKind kind;
uint32_t idx;

explicit Record(rir::TypeFeedbackKind kind, unsigned idx);
explicit Record(rir::TypeFeedback* feedback, rir::TypeFeedbackKind kind,
unsigned idx);
Value* getValue() const;
void setValue(Value* callee);
void printArgs(std::ostream& out, bool tty) const override;
Expand Down
19 changes: 13 additions & 6 deletions rir/src/compiler/rir2pir/rir2pir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,9 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos,
Value* target = top();

if (baseline) {
auto rec = insert(new Record(rir::TypeFeedbackKind::Test, idx));
auto feedback = &srcCode->function()->typeFeedback();
auto rec =
insert(new Record(feedback, rir::TypeFeedbackKind::Test, idx));
rec->setValue(target);
} else {
auto& feedback = typeFeedback.test(idx);
Expand Down Expand Up @@ -413,7 +415,9 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos,
Value* target = top();

if (baseline) {
auto rec = insert(new Record(rir::TypeFeedbackKind::Type, idx));
auto feedback = &srcCode->function()->typeFeedback();
auto rec =
insert(new Record(feedback, rir::TypeFeedbackKind::Type, idx));
rec->setValue(target);
} else {
auto& feedback = typeFeedback.types(idx);
Expand Down Expand Up @@ -457,7 +461,9 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos,
Value* target = top();

if (baseline) {
auto rec = insert(new Record(rir::TypeFeedbackKind::Call, idx));
auto feedback = &srcCode->function()->typeFeedback();
auto rec =
insert(new Record(feedback, rir::TypeFeedbackKind::Call, idx));
rec->setValue(target);
} else {
const auto& feedback = typeFeedback.callees(bc.immediate.i);
Expand Down Expand Up @@ -490,7 +496,7 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos,
if (feedback.numTargets == 1) {
assert(!feedback.invalid &&
"feedback can't be invalid if numTargets is 1");
f.monomorphic = feedback.getTarget(srcCode, 0);
f.monomorphic = feedback.getTarget(srcCode->function(), 0);
f.type = TYPEOF(f.monomorphic);
f.stableEnv = true;
} else if (feedback.numTargets > 1) {
Expand All @@ -499,7 +505,7 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos,
bool stableBody = !feedback.invalid;
bool stableEnv = !feedback.invalid;
for (size_t i = 0; i < feedback.numTargets; ++i) {
SEXP b = feedback.getTarget(srcCode, i);
SEXP b = feedback.getTarget(srcCode->function(), i);
if (!first) {
first = b;
} else {
Expand All @@ -524,7 +530,8 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos,
d->callTargetTrigger();
for (size_t i = 0; i < feedback.numTargets;
++i) {
SEXP b = feedback.getTarget(srcCode, i);
SEXP b = feedback.getTarget(
srcCode->function(), i);
if (b != deoptCallTarget)
deoptedCallTargets.insert(b);
}
Expand Down
16 changes: 10 additions & 6 deletions rir/src/runtime/TypeFeedback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ void ObservedCallees::record(Code* caller, SEXP callee,
}
}

SEXP ObservedCallees::getTarget(const Code* code, size_t pos) const {
SEXP ObservedCallees::getTarget(const Function* function, size_t pos) const {
assert(pos < numTargets);
return code->getExtraPoolEntry(targets[pos]);
return function->body()->getExtraPoolEntry(targets[pos]);
}

FeedbackOrigin::FeedbackOrigin(rir::Function* function, uint32_t idx)
Expand Down Expand Up @@ -87,7 +87,7 @@ void DeoptReason::record(SEXP val) const {
}
}

void ObservedCallees::print(std::ostream& out, const Code* code) const {
void ObservedCallees::print(std::ostream& out, const Function* function) const {
out << "callees: ";
if (taken == ObservedCallees::CounterOverflow)
out << "*, <";
Expand All @@ -102,7 +102,7 @@ void ObservedCallees::print(std::ostream& out, const Code* code) const {
out << (numTargets ? ", " : " ");

for (unsigned i = 0; i < numTargets; ++i) {
auto target = getTarget(code, i);
auto target = getTarget(function, i);
out << target << "(" << Rf_type2char(TYPEOF(target)) << ") ";
}
}
Expand Down Expand Up @@ -170,7 +170,7 @@ void TypeFeedbackSlot::print(std::ostream& out,
const Function* function) const {
switch (kind) {
case TypeFeedbackKind::Call:
feedback_.callees.print(out, function->body());
feedback_.callees.print(out, function);
break;
case TypeFeedbackKind::Test:
feedback_.test.print(out);
Expand All @@ -182,7 +182,8 @@ void TypeFeedbackSlot::print(std::ostream& out,
}

void TypeFeedback::print(std::ostream& out) const {
std::cout << "== type feedback ==" << std::endl;
std::cout << "== type feedback " << this << " (fun " << owner_
<< ") ==" << std::endl;
int i = 0;
for (auto& slot : slots_) {
out << "#" << i++ << ": ";
Expand Down Expand Up @@ -214,4 +215,7 @@ TypeFeedbackSlot* FeedbackOrigin::slot() const {
return nullptr;
}
}
bool FeedbackOrigin::isValid() const {
return function_ != nullptr && function_->typeFeedback().size() > idx_;
}
} // namespace rir
8 changes: 5 additions & 3 deletions rir/src/runtime/TypeFeedback.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ struct ObservedCallees {
std::array<unsigned, MaxTargets> targets;

void record(Code* caller, SEXP callee, bool invalidateWhenFull = false);
SEXP getTarget(const Code* code, size_t pos) const;
void print(std::ostream& out, const Code* code) const;
SEXP getTarget(const Function* function, size_t pos) const;
void print(std::ostream& out, const Function* function) const;
};

static_assert(sizeof(ObservedCallees) == 4 * sizeof(uint32_t),
Expand Down Expand Up @@ -154,7 +154,7 @@ struct FeedbackOrigin {
FeedbackOrigin() {}
FeedbackOrigin(rir::Function* fun, uint32_t idx);

bool isValid() const { return function_ != nullptr; }
bool isValid() const;
TypeFeedbackSlot* slot() const;
uint32_t idx() const { return idx_; }
Function* function() const { return function_; }
Expand Down Expand Up @@ -324,6 +324,8 @@ class TypeFeedback {
void print(std::ostream& out) const;

void record(uint32_t idx, SEXP callee);

uint32_t size() const { return slots_.size(); }
};

#pragma pack(pop)
Expand Down