From f027a8b624f89b18e56c1f0dd90291fb20f10eac Mon Sep 17 00:00:00 2001 From: Sebastian Krynski Date: Thu, 2 Jun 2022 09:48:30 +0000 Subject: [PATCH 1/2] do we benefit from stable envs? --- rir/src/compiler/rir2pir/rir2pir.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rir/src/compiler/rir2pir/rir2pir.cpp b/rir/src/compiler/rir2pir/rir2pir.cpp index 49309d7c1..92cd0ebc1 100644 --- a/rir/src/compiler/rir2pir/rir2pir.cpp +++ b/rir/src/compiler/rir2pir/rir2pir.cpp @@ -519,6 +519,8 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos, f.monomorphic = first; if (stableEnv) f.stableEnv = true; + + f.stableEnv = false; } } break; From 96dd5c0c8913da84f87a20c5ce2860877195cf22 Mon Sep 17 00:00:00 2001 From: Sebastian Krynski Date: Thu, 9 Jun 2022 16:08:12 +0000 Subject: [PATCH 2/2] measure perf --- .vscode/settings.json | 3 +- rir/src/compiler/pir/instruction.h | 1 + rir/src/compiler/rir2pir/rir2pir.cpp | 40 ++++++++++++++++++++++++-- rir/src/compiler/test/PirCheck.cpp | 8 +++++- rir/src/compiler/util/bb_transform.cpp | 3 ++ rir/src/runtime/TypeFeedback.cpp | 10 +++++++ 6 files changed, 60 insertions(+), 5 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 3c989f58d..d4dc74778 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -112,6 +112,7 @@ "cfenv": "cpp", "csignal": "cpp", "__functional_base_03": "cpp", - "__memory": "cpp" + "__memory": "cpp", + "version": "cpp" } } diff --git a/rir/src/compiler/pir/instruction.h b/rir/src/compiler/pir/instruction.h index 63739a3c0..bbbbd8b86 100644 --- a/rir/src/compiler/pir/instruction.h +++ b/rir/src/compiler/pir/instruction.h @@ -154,6 +154,7 @@ struct CallFeedback { SEXP monomorphic = nullptr; SEXPTYPE type = NILSXP; bool stableEnv = false; + bool stableEnvHint = false; }; class DominanceGraph; diff --git a/rir/src/compiler/rir2pir/rir2pir.cpp b/rir/src/compiler/rir2pir/rir2pir.cpp index 92cd0ebc1..78a2c5f9f 100644 --- a/rir/src/compiler/rir2pir/rir2pir.cpp +++ b/rir/src/compiler/rir2pir/rir2pir.cpp @@ -462,9 +462,33 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos, f.taken = feedback.taken; f.feedbackOrigin = FeedbackOrigin(srcCode, pos); if (feedback.numTargets == 1) { - f.monomorphic = feedback.getTarget(srcCode, 0); + + // if (auto ld = LdFun::Cast(i)) { + // // if ( ld->varName == Rf_install("f")) { + // // int a=3; + // // //assert(false && "f!"); + + // // } + + // } + + auto target = feedback.getTarget(srcCode, 0); + + if (TYPEOF(target) == CLOSXP) { + if (!isValidClosureSEXP(target)) { + + rir::Compiler::compileClosure(target); + assert(isValidClosureSEXP(target)); + f.stableEnv = false; + } + } else { + f.stableEnv = true; + } + f.stableEnvHint = true; + // f.stableEnv =true; + f.monomorphic = target; f.type = TYPEOF(f.monomorphic); - f.stableEnv = true; + } else if (feedback.numTargets > 1) { SEXP first = nullptr; bool stableType = true; @@ -604,8 +628,18 @@ bool Rir2Pir::compileBC(const BC& bc, Opcode* pos, Opcode* nextPos, if (ldfun) { if (ti.monomorphic) { ldfun->hint(ti.monomorphic, ti.feedbackOrigin); - if (ti.stableEnv) + if (ti.stableEnvHint) { ldfun->hintHasStableEnv = true; + + if (TYPEOF(ti.monomorphic) == CLOSXP) { + + if (!isValidClosureSEXP(ti.monomorphic)) { + + rir::Compiler::compileClosure(ti.monomorphic); + assert(isValidClosureSEXP(ti.monomorphic)); + } + } + } } else { ldfun->hint(symbol::ambiguousCallTarget, {}); } diff --git a/rir/src/compiler/test/PirCheck.cpp b/rir/src/compiler/test/PirCheck.cpp index cd0c6fc93..ff3ccb2e1 100644 --- a/rir/src/compiler/test/PirCheck.cpp +++ b/rir/src/compiler/test/PirCheck.cpp @@ -90,7 +90,13 @@ static bool testNoPromise(ClosureVersion* f) { static bool testNoExternalCalls(ClosureVersion* f) { return Visitor::check(f->entry, [&](Instruction* i) { - return !CallInstruction::CastCall(i) || CallSafeBuiltin::Cast(i); + auto res = !CallInstruction::CastCall(i) || CallSafeBuiltin::Cast(i); + if (!res && f->owner()) { + i->print(std::cout, true); + std::cerr << i; + // assert(false); + } + return res; }); } diff --git a/rir/src/compiler/util/bb_transform.cpp b/rir/src/compiler/util/bb_transform.cpp index b6a013044..5b043ca37 100644 --- a/rir/src/compiler/util/bb_transform.cpp +++ b/rir/src/compiler/util/bb_transform.cpp @@ -336,6 +336,9 @@ Value* BBTransform::insertCalleeGuard(Compiler& compiler, : (stableEnv ? compiler.module->c(fb.monomorphic) : compiler.module->c(BODY(fb.monomorphic))); + // if (expected == nullptr || expected == Nil::instance() ) + // assert(false && "pir nil"); + auto t = new Identical(calleeForGuard, expected, PirType::any()); pos = bb->insert(pos, t) + 1; diff --git a/rir/src/runtime/TypeFeedback.cpp b/rir/src/runtime/TypeFeedback.cpp index c99ec88bf..d32d1f35c 100644 --- a/rir/src/runtime/TypeFeedback.cpp +++ b/rir/src/runtime/TypeFeedback.cpp @@ -13,10 +13,20 @@ void ObservedCallees::record(Code* caller, SEXP callee) { if (taken < CounterOverflow) taken++; if (numTargets < MaxTargets) { + + if (TYPEOF(callee) != CLOSXP && TYPEOF(callee) != BUILTINSXP && + TYPEOF(callee) != SPECIALSXP) { + + // Rf_PrintValue(callee); + // assert(false && "aa"); + return; + } + int i = 0; for (; i < numTargets; ++i) if (caller->getExtraPoolEntry(targets[i]) == callee) break; + if (i == numTargets) { auto idx = caller->addExtraPoolEntry(callee); targets[numTargets++] = idx;