Skip to content

Commit 6e67f62

Browse files
JanJecmenJanJecmen
committed
try
1 parent 0fd9acb commit 6e67f62

File tree

15 files changed

+301
-191
lines changed

15 files changed

+301
-191
lines changed

‎rir/src/compiler/analysis/verifier.cpp‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class TheVerifier {
3535
std::unordered_set<BB*> seenPreds;
3636

3737
void operator()() {
38-
Visitor::run(f->entry, [&](BB* bb) { return verify(bb, false); });
38+
Visitor::run(f->entry, [&](BB* bb) { return verify(bb); });
3939
Visitor::run(f->entry, [&](BB* bb) { seenPreds.erase(bb); });
4040
if (!seenPreds.empty()) {
4141
std::cerr << "The following preds are not reachable from entry: ";
@@ -98,7 +98,7 @@ class TheVerifier {
9898
return doms.at(c);
9999
}
100100

101-
void verify(BB* bb, bool inPromise) {
101+
void verify(BB* bb) {
102102
if (bb->id >= bb->owner->nextBBId) {
103103
std::cout << "BB" << bb->id << " id is bigger than max ("
104104
<< bb->owner->nextBBId << ")\n";
@@ -121,7 +121,7 @@ class TheVerifier {
121121
}
122122

123123
for (auto i : *bb) {
124-
verify(i, bb, inPromise);
124+
verify(i, bb);
125125
}
126126
/* This check verifies that our graph is in edge-split format.
127127
Currently we do not rely on this property, however we should
@@ -196,10 +196,10 @@ class TheVerifier {
196196
}
197197

198198
void verify(Promise* p) {
199-
Visitor::run(p->entry, [&](BB* bb) { verify(bb, true); });
199+
Visitor::run(p->entry, [&](BB* bb) { verify(bb); });
200200
}
201201

202-
void verify(Instruction* i, BB* bb, bool inPromise) {
202+
void verify(Instruction* i, BB* bb) {
203203
if (i->bb() != bb) {
204204
std::cerr << "Error: instruction '";
205205
i->print(std::cerr);
@@ -269,19 +269,6 @@ class TheVerifier {
269269
});
270270
}
271271

272-
if (i->frameState()) {
273-
if (!inPromise) {
274-
auto fs = i->frameState();
275-
while (fs->next())
276-
fs = fs->next();
277-
if (fs->inPromise) {
278-
std::cerr << "Error: instruction '";
279-
i->print(std::cerr);
280-
std::cerr << "' outermost fs inPromis in body code\n";
281-
ok = false;
282-
}
283-
}
284-
}
285272
if (auto assume = Assume::Cast(i)) {
286273
if (IsType::Cast(assume->arg(0).val())) {
287274
if (!assume->reason.pc()) {

‎rir/src/compiler/compiler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ void Compiler::compileClosure(Closure* closure, rir::Function* optFunction,
155155
auto arg = closure->formals().defaultArgs()[idx];
156156
assert(rir::Code::check(arg) && "Default arg not compiled");
157157
auto code = rir::Code::unpack(arg);
158-
auto res = rir2pir.tryCreateArg(code, builder, false);
158+
auto res = rir2pir.tryCreateArg(code, builder);
159159
if (!res) {
160160
failedToCompileDefaultArgs = true;
161161
return;

‎rir/src/compiler/native/builtins.cpp‎

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -828,8 +828,8 @@ static SEXP deoptSentinelContainer = []() {
828828
return store;
829829
}();
830830

831-
void deoptImpl(rir::Code* c, SEXP cls, DeoptMetadata* m, R_bcstack_t* args,
832-
bool leakedEnv, DeoptReason* deoptReason, SEXP deoptTrigger) {
831+
SEXP deopt(rir::Code* c, SEXP cls, DeoptMetadata* m, R_bcstack_t* args,
832+
bool leakedEnv, DeoptReason* deoptReason, SEXP deoptTrigger) {
833833
deoptReason->record(deoptTrigger);
834834

835835
assert(m->numFrames >= 1);
@@ -854,8 +854,9 @@ void deoptImpl(rir::Code* c, SEXP cls, DeoptMetadata* m, R_bcstack_t* args,
854854
auto le = LazyEnvironment::check(env);
855855
if (deoptless && m->numFrames == 1 && cls != deoptlessRecursion &&
856856
((le && !le->materialized()) ||
857-
(!le && (!leakedEnv || !deoptlessNoLeakedEnvs)))) {
858-
assert(m->frames[0].inPromise == false);
857+
(!le && (!leakedEnv || !deoptlessNoLeakedEnvs))) &&
858+
/* TODO: support deoptless when outermost frame is a promise */
859+
!m->frames[0].inPromise) {
859860

860861
size_t envSize = le ? le->nargs : Rf_length(FRAME(env));
861862
if (envSize <= DeoptContext::MAX_ENV &&
@@ -932,8 +933,8 @@ void deoptImpl(rir::Code* c, SEXP cls, DeoptMetadata* m, R_bcstack_t* args,
932933

933934
Rf_findcontext(CTXT_BROWSER | CTXT_FUNCTION,
934935
originalCntxt->cloenv, res);
935-
assert(false);
936-
return;
936+
assert(false && "unreachable after deoptless");
937+
return nullptr;
937938
}
938939
}
939940
}
@@ -945,13 +946,35 @@ void deoptImpl(rir::Code* c, SEXP cls, DeoptMetadata* m, R_bcstack_t* args,
945946
if (f->body() == c)
946947
Pool::patch(idx, deoptSentinelContainer);
947948

948-
CallContext call(ArglistOrder::NOT_REORDERED, c, cls,
949-
/* nargs */ -1, src_pool_at(c->src), args,
950-
(Immediate*)nullptr, env, R_NilValue, Context());
949+
if (cls) {
950+
CallContext call(ArglistOrder::NOT_REORDERED, c, cls,
951+
/* nargs */ -1, src_pool_at(c->src), args,
952+
(Immediate*)nullptr, env, R_NilValue, Context());
951953

952-
deoptFramesWithContext(&call, m, R_NilValue, m->numFrames - 1, stackHeight,
953-
(RCNTXT*)R_GlobalContext);
954-
assert(false);
954+
// Deopt in a function longjumps to its context
955+
deoptFramesWithContext(&call, m, R_NilValue, m->numFrames - 1,
956+
stackHeight, (RCNTXT*)R_GlobalContext);
957+
assert(false && "unreachable after deopt");
958+
return nullptr;
959+
} else {
960+
// Deopt in a promise has nowhere to longjump, so it leaves the result
961+
// on the TOS and returns here, this is immediately returned as the
962+
// result of the promise
963+
deoptFramesWithContext(nullptr, m, R_NilValue, m->numFrames - 1,
964+
stackHeight, (RCNTXT*)R_GlobalContext);
965+
return ostack_pop();
966+
}
967+
}
968+
969+
void deoptImpl(rir::Code* c, SEXP cls, DeoptMetadata* m, R_bcstack_t* args,
970+
bool leakedEnv, DeoptReason* deoptReason, SEXP deoptTrigger) {
971+
deopt(c, cls, m, args, leakedEnv, deoptReason, deoptTrigger);
972+
}
973+
974+
SEXP deoptPromImpl(rir::Code* c, DeoptMetadata* m, R_bcstack_t* args,
975+
bool leakedEnv, DeoptReason* deoptReason,
976+
SEXP deoptTrigger) {
977+
return deopt(c, nullptr, m, args, leakedEnv, deoptReason, deoptTrigger);
955978
}
956979

957980
void recordTypefeedbackImpl(Opcode* pos, rir::Code* code, SEXP value) {
@@ -2437,6 +2460,14 @@ void NativeBuiltins::initializeBuiltins() {
24372460
t::DeoptReasonPtr, t::SEXP},
24382461
false),
24392462
{llvm::Attribute::NoReturn}};
2463+
get_(Id::deoptProm) = {
2464+
"deoptProm",
2465+
(void*)&deoptPromImpl,
2466+
llvm::FunctionType::get(t::SEXP,
2467+
{t::voidPtr, t::voidPtr, t::stackCellPtr, t::i1,
2468+
t::DeoptReasonPtr, t::SEXP},
2469+
false),
2470+
{}};
24402471
get_(Id::assertFail) = {"assertFail",
24412472
(void*)&assertFailImpl,
24422473
t::void_voidPtr,

‎rir/src/compiler/native/builtins.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ struct NativeBuiltins {
8787
length,
8888
recordTypefeedback,
8989
deopt,
90+
deoptProm,
9091
assertFail,
9192
printValue,
9293
extract11,

‎rir/src/compiler/native/lower_function_llvm.cpp‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,6 +2112,7 @@ void LowerFunctionLLVM::compile() {
21122112
stack({container(paramCode())});
21132113
additionalStackSlots++;
21142114
}
2115+
21152116
{
21162117
SmallSet<std::pair<Value*, SEXP>> bindings;
21172118
Visitor::run(code->entry, [&](Instruction* i) {
@@ -3555,21 +3556,42 @@ void LowerFunctionLLVM::compile() {
35553556
args.push_back(fs->arg(pos).val());
35563557
args.push_back(fs->env());
35573558
m->frames[frameNr--] = {fs->pc, fs->code, fs->stackSize,
3558-
fs->inPromise};
3559+
fs->inPromise, fs->patch};
35593560
}
35603561

35613562
target->addExtraPoolEntry(store);
35623563
}
35633564

3564-
withCallFrame(args, [&]() {
3565-
return call(NativeBuiltins::get(NativeBuiltins::Id::deopt),
3566-
{paramCode(), paramClosure(),
3567-
convertToPointer(m, t::i8, true), paramArgs(),
3568-
c(deopt->escapedEnv, 1),
3569-
load(deopt->deoptReason()),
3570-
loadSxp(deopt->deoptTrigger())});
3571-
});
3572-
builder.CreateUnreachable();
3565+
// Deopt only returns if the outermost frame is a promise.
3566+
// In that case, the result is the returned value and we simply
3567+
// return it from here.
3568+
if (code->isPromise()) {
3569+
auto res = withCallFrame(
3570+
args,
3571+
[&]() {
3572+
return call(NativeBuiltins::get(
3573+
NativeBuiltins::Id::deoptProm),
3574+
{paramCode(),
3575+
convertToPointer(m, t::i8, true),
3576+
paramArgs(), c(deopt->escapedEnv, 1),
3577+
load(deopt->deoptReason()),
3578+
loadSxp(deopt->deoptTrigger())});
3579+
},
3580+
false);
3581+
exitBlocks.push_back(builder.GetInsertBlock());
3582+
builder.CreateRet(res);
3583+
} else {
3584+
withCallFrame(args, [&]() {
3585+
return call(
3586+
NativeBuiltins::get(NativeBuiltins::Id::deopt),
3587+
{paramCode(), paramClosure(),
3588+
convertToPointer(m, t::i8, true), paramArgs(),
3589+
c(deopt->escapedEnv, 1),
3590+
load(deopt->deoptReason()),
3591+
loadSxp(deopt->deoptTrigger())});
3592+
});
3593+
builder.CreateUnreachable();
3594+
}
35733595
break;
35743596
}
35753597

‎rir/src/compiler/opt/force_dominance.cpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,14 @@ bool ForceDominance::apply(Compiler&, ClosureVersion* cls, Code* code,
192192
Value* eager = mkarg->eagerArg();
193193
f->replaceUsesWith(eager);
194194
next = bb->remove(ip);
195-
} else if (toInline.count(f)) {
195+
} else if (toInline.count(f) &&
196+
// Can't inline a promise with Assumes if the
197+
// dominating Force doesn't have a Framestate
198+
(f->frameState() ||
199+
Visitor::check(mkarg->prom()->entry,
200+
[](Instruction* i) {
201+
return !Assume::Cast(i);
202+
}))) {
196203
anyChange = true;
197204
Promise* prom = mkarg->prom();
198205
BB* split =

‎rir/src/compiler/pir/builder.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ void Builder::add(Instruction* i) {
6060
}
6161

6262
FrameState* Builder::registerFrameState(rir::Code* srcCode, Opcode* pos,
63-
const RirStack& stack, bool inPromise) {
64-
auto sp = new FrameState(env, srcCode, pos, stack, inPromise);
63+
const RirStack& stack, bool inPromise, int8_t patch) {
64+
auto sp = new FrameState(env, srcCode, pos, stack, inPromise, patch);
6565
add(sp);
6666
return sp;
6767
}

‎rir/src/compiler/pir/builder.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class Builder {
4545
void setBranch(BB* bb1, BB* bb2);
4646

4747
FrameState* registerFrameState(rir::Code* srcCode, Opcode* pos,
48-
const RirStack& stack, bool inPromise);
48+
const RirStack& stack, bool inPromise, int8_t patch = -1);
4949
Checkpoint* emitCheckpoint(rir::Code* srcCode, Opcode* pos,
5050
const RirStack& stack, bool inPromise);
5151
Checkpoint* emitCheckpoint(FrameState* fs);

‎rir/src/compiler/pir/code.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ class Code {
3434
virtual rir::Code* rirSrc() const = 0;
3535
virtual void printName(std::ostream&) const = 0;
3636

37+
virtual bool isPromise() const { return false; }
38+
3739
friend std::ostream& operator<<(std::ostream& out, const Code& e) {
3840
e.printName(out);
3941
return out;

‎rir/src/compiler/pir/instruction.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,7 @@ class VLIE(FrameState, Effects() | Effect::ReadsEnv) {
928928
rir::Code* code;
929929
size_t stackSize;
930930
bool inPromise;
931+
int8_t patch;
931932

932933
size_t gvnBase() const override {
933934
return hash_combine(
@@ -937,9 +938,9 @@ class VLIE(FrameState, Effects() | Effect::ReadsEnv) {
937938
}
938939

939940
FrameState(Value* env, rir::Code* code, Opcode* pc, const RirStack& stack,
940-
bool inPromise)
941+
bool inPromise, int8_t patch)
941942
: VarLenInstructionWithEnvSlot(NativeType::frameState, env), pc(pc),
942-
code(code), stackSize(stack.size()), inPromise(inPromise) {
943+
code(code), stackSize(stack.size()), inPromise(inPromise), patch(patch) {
943944
for (auto& v : stack)
944945
pushArg(v, PirType::any());
945946
}

0 commit comments

Comments
 (0)