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.

Sign up for GitHub

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

Jump to bottom

create ifuncs for compiled blocks once at module load time #4905

Merged
merged 12 commits into from Nov 28, 2021
Merged

create ifuncs for compiled blocks once at module load time #4905

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2507cb5
build block ifunc eagerly at all necessary call sites
froydnj Nov 22, 2021
bed7053
eagerly synthesize a block ifunc for sorbet_i_send
froydnj Nov 22, 2021
36b5c7d
refactor code to build ifuncs in Payload only
froydnj Nov 22, 2021
b2bde9d
only create the ifunc structure once at module load time
froydnj Nov 23, 2021
9b32902
cache the ifunc structure as a global GC'able constant
froydnj Nov 23, 2021
1e452f5
enable now-working test
froydnj Nov 23, 2021
990b512
update compiler exp
froydnj Nov 23, 2021
926e39f
format C++
froydnj Nov 23, 2021
a9209aa
s/buildBlockIfunc/getOrBuildBlockIfunc/
froydnj Nov 28, 2021
088d660
format C++
froydnj Nov 28, 2021
b3b46f6
slightly less zealous s///
froydnj Nov 28, 2021
4a7d5fe
be even less zealous
froydnj Nov 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 7 additions & 14 deletions compiler/IREmitter/NameBasedIntrinsics.cc
View file
Open in desktop
Expand Up @@ -491,15 +491,9 @@ class CallWithSplat : public NameBasedIntrinsicMethod {
auto *kwSplatFlag = llvm::ConstantInt::get(cs, llvm::APInt(32, flags.kw_splat ? 1 : 0));

if (auto *blk = mcctx.blkAsFunction()) {
auto arity = irctx.rubyBlockArity[mcctx.blk.value()];
auto *blkMinArgs = IREmitterHelpers::buildS4(cs, arity.min);
auto *blkMaxArgs = IREmitterHelpers::buildS4(cs, arity.max);

// blocks require a locals offset parameter
llvm::Value *localsOffset = Payload::buildLocalsOffset(cs);
ENFORCE(localsOffset != nullptr);
return builder.CreateCall(cs.getFunction("sorbet_callSuperBlock"),
{argc, argv, kwSplatFlag, blk, blkMinArgs, blkMaxArgs, localsOffset},
auto blkId = mcctx.blk.value();
auto *ifunc = Payload::buildBlockIfunc(cs, builder, irctx, blkId);
return builder.CreateCall(cs.getFunction("sorbet_callSuperBlock"), {argc, argv, kwSplatFlag, ifunc},
"rawSendResult");
} else {
return builder.CreateCall(cs.getFunction("sorbet_callSuper"), {argc, argv, kwSplatFlag},
Expand All @@ -519,11 +513,10 @@ class CallWithSplat : public NameBasedIntrinsicMethod {

// Call the receiver.
if (auto *blk = mcctx.blkAsFunction()) {
auto *closure = Payload::buildLocalsOffset(cs);
auto arity = irctx.rubyBlockArity[mcctx.blk.value()];
auto usesBreak = irctx.blockUsesBreak[mcctx.blk.value()];
return Payload::callFuncBlockWithCache(mcctx.cs, mcctx.builder, cache, usesBreak, blk, arity.min,
arity.max, closure);
auto blkId = mcctx.blk.value();
auto usesBreak = irctx.blockUsesBreak[blkId];
auto *ifunc = Payload::buildBlockIfunc(cs, builder, irctx, blkId);
return Payload::callFuncBlockWithCache(mcctx.cs, mcctx.builder, cache, usesBreak, ifunc);
} else {
auto *blockHandler = Payload::vmBlockHandlerNone(mcctx.cs, mcctx.builder);
return Payload::callFuncWithCache(mcctx.cs, mcctx.builder, cache, blockHandler);
Expand Down
47 changes: 39 additions & 8 deletions compiler/IREmitter/Payload.cc
View file
Open in desktop
Expand Up @@ -1115,16 +1115,12 @@ llvm::Value *Payload::callFuncWithCache(CompilerState &cs, llvm::IRBuilderBase &
}

llvm::Value *Payload::callFuncBlockWithCache(CompilerState &cs, llvm::IRBuilderBase &builder, llvm::Value *cache,
bool usesBreak, llvm::Value *blockFun, int blkMinArgs, int blkMaxArgs,
llvm::Value *closure) {
auto *minArgs = IREmitterHelpers::buildS4(cs, blkMinArgs);
auto *maxArgs = IREmitterHelpers::buildS4(cs, blkMaxArgs);
bool usesBreak, llvm::Value *ifunc) {
if (usesBreak) {
return builder.CreateCall(cs.getFunction("sorbet_callFuncBlockWithCache"),
{cache, blockFun, minArgs, maxArgs, closure}, "sendWithBlock");
return builder.CreateCall(cs.getFunction("sorbet_callFuncBlockWithCache"), {cache, ifunc}, "sendWithBlock");
} else {
return builder.CreateCall(cs.getFunction("sorbet_callFuncBlockWithCache_noBreak"),
{cache, blockFun, minArgs, maxArgs, closure}, "sendWithBlock");
return builder.CreateCall(cs.getFunction("sorbet_callFuncBlockWithCache_noBreak"), {cache, ifunc},
"sendWithBlock");
}
}

Expand Down Expand Up @@ -1165,4 +1161,39 @@ llvm::Value *Payload::getCFPForBlock(CompilerState &cs, llvm::IRBuilderBase &bui
llvm::Value *Payload::buildLocalsOffset(CompilerState &cs) {
return llvm::ConstantInt::get(cs, llvm::APInt(64, 0, true));
}

llvm::Value *Payload::buildBlockIfunc(CompilerState &cs, llvm::IRBuilderBase &builder, const IREmitterContext &irctx,
Copy link
Contributor

Choose a reason for hiding this comment

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

I might suggest renaming to getOrBuildBlockIfunc. Working backwards from the call sites I found myself a bit confused that we were "building" an ifunc at certain places, but now I see that this actually uses getOrInsertGlobal.

int blkId) {
auto *blk = irctx.rubyBlocks2Functions[blkId];
auto &arity = irctx.rubyBlockArity[blkId];
auto rawName = fmt::format("{}_ifunc", (string)blk->getName());

auto *globalTy = llvm::Type::getInt64Ty(cs);

auto *global = cs.module->getOrInsertGlobal(rawName, globalTy, [&cs, &blk, &rawName, &globalTy, &arity]() {
auto globalInitBuilder = llvm::IRBuilder<>(cs);

auto isConstant = false;
auto *zero = llvm::ConstantInt::get(cs, llvm::APInt(64, 0));
auto global = new llvm::GlobalVariable(*cs.module, globalTy, isConstant, llvm::GlobalVariable::InternalLinkage,
zero, rawName);

globalInitBuilder.SetInsertPoint(cs.globalConstructorsEntry);

auto *blkMinArgs = IREmitterHelpers::buildS4(cs, arity.min);
auto *blkMaxArgs = IREmitterHelpers::buildS4(cs, arity.max);
auto *offset = buildLocalsOffset(cs);
auto *ifunc = globalInitBuilder.CreateCall(cs.getFunction("sorbet_buildBlockIfunc"),
{blk, blkMinArgs, blkMaxArgs, offset});
auto *asValue = globalInitBuilder.CreateBitOrPointerCast(ifunc, globalTy);
auto *globalIndex = globalInitBuilder.CreateCall(cs.getFunction("sorbet_globalConstRegister"), {asValue});
globalInitBuilder.CreateStore(globalIndex, global);

return global;
});

auto *globalIndex = builder.CreateLoad(global);
return builder.CreateCall(cs.getFunction("sorbet_globalConstFetchIfunc"), {globalIndex});
}

} // namespace sorbet::compiler
6 changes: 4 additions & 2 deletions compiler/IREmitter/Payload.h
View file
Open in desktop
Expand Up @@ -103,8 +103,7 @@ class Payload {
static llvm::Value *callFuncWithCache(CompilerState &cs, llvm::IRBuilderBase &builder, llvm::Value *cache,
llvm::Value *blockHandler);
static llvm::Value *callFuncBlockWithCache(CompilerState &cs, llvm::IRBuilderBase &builder, llvm::Value *cache,
bool usesBreak, llvm::Value *blockFun, int blkMinArgs, int blkMaxArgs,
llvm::Value *closure);
bool usesBreak, llvm::Value *ifunc);
static llvm::Value *callFuncDirect(CompilerState &cs, llvm::IRBuilderBase &builder, llvm::Value *cache,
llvm::Value *fn, llvm::Value *argc, llvm::Value *argv, llvm::Value *recv,
llvm::Value *iseq);
Expand All @@ -120,6 +119,9 @@ class Payload {
int rubyBlockId);

static llvm::Value *buildLocalsOffset(CompilerState &cs);

static llvm::Value *buildBlockIfunc(CompilerState &cs, llvm::IRBuilderBase &builder, const IREmitterContext &irctx,
int blkId);
};
} // namespace sorbet::compiler
#endif
35 changes: 18 additions & 17 deletions compiler/IREmitter/Payload/codegen-payload.c
View file
Open in desktop
Expand Up @@ -132,8 +132,7 @@ SORBET_ALIVE(VALUE, sorbet_i_getRubyClass, (const char *const className, long cl
SORBET_ALIVE(VALUE, sorbet_i_getRubyConstant, (const char *const className, long classNameLen) __attribute__((const)));
SORBET_ALIVE(VALUE, sorbet_i_objIsKindOf, (VALUE, VALUE));
SORBET_ALIVE(VALUE, sorbet_i_send,
(struct FunctionInlineCache *, _Bool blkUsesBreak, BlockFFIType blk, int blkMinArgs, int blkMaxArgs, VALUE,
rb_control_frame_t *, ...));
(struct FunctionInlineCache *, _Bool blkUsesBreak, struct vm_ifunc *, rb_control_frame_t *, ...));

SORBET_ALIVE(_Bool, sorbet_i_isa_Integer, (VALUE) __attribute__((const)));
SORBET_ALIVE(_Bool, sorbet_i_isa_TrueClass, (VALUE) __attribute__((const)));
Expand All @@ -153,6 +152,7 @@ SORBET_ALIVE(_Bool, sorbet_i_isa_RootSingleton, (VALUE) __attribute__((const)));

SORBET_ALIVE(long, sorbet_globalConstRegister, (VALUE val));
SORBET_ALIVE(VALUE, sorbet_globalConstDupHash, (long index));
SORBET_ALIVE(struct vm_ifunc *, sorbet_globalConstFetchIfunc, (long index));
SORBET_ALIVE(VALUE, sorbet_magic_mergeHashHelper, (VALUE, VALUE));

SORBET_ALIVE(VALUE, sorbet_vm_getivar, (VALUE obj, ID id, struct iseq_inline_iv_cache_entry *cache));
Expand Down Expand Up @@ -215,8 +215,7 @@ SORBET_ALIVE(VALUE, sorbet_run_exception_handling,
// The special value indicating that we need to retry.
VALUE retrySingleton, long exceptionValueIndex, long exceptionValueLevel));

SORBET_ALIVE(VALUE, sorbet_rb_iterate,
(VALUE(*body)(VALUE), VALUE data1, rb_block_call_func_t bl_proc, int minArgs, int maxArgs, VALUE data2));
SORBET_ALIVE(VALUE, sorbet_rb_iterate, (VALUE(*body)(VALUE), VALUE data1, const struct vm_ifunc *ifunc));
SORBET_ALIVE(VALUE, sorbet_vm_aref,
(rb_control_frame_t * cfp, struct FunctionInlineCache *cache, VALUE recv, VALUE arg));
SORBET_ALIVE(VALUE, sorbet_vm_plus,
Expand Down Expand Up @@ -2257,23 +2256,26 @@ static VALUE sorbet_iterMethod(VALUE obj) {
}

SORBET_INLINE
VALUE sorbet_callFuncBlockWithCache(struct FunctionInlineCache *cache, BlockFFIType blockImpl, int blkMinArgs,
int blkMaxArgs, VALUE closure) {
return sorbet_rb_iterate(sorbet_iterMethod, (VALUE)cache, blockImpl, blkMinArgs, blkMaxArgs, closure);
const struct vm_ifunc *sorbet_buildBlockIfunc(BlockFFIType blockImpl, int blkMinArgs, int blkMaxArgs, VALUE closure) {
return rb_vm_ifunc_new(blockImpl, (void *)closure, blkMinArgs, blkMaxArgs);
}
KEEP_ALIVE(sorbet_buildBlockIfunc);

SORBET_INLINE
VALUE sorbet_callFuncBlockWithCache(struct FunctionInlineCache *cache, const struct vm_ifunc *ifunc) {
return sorbet_rb_iterate(sorbet_iterMethod, (VALUE)cache, ifunc);
}
KEEP_ALIVE(sorbet_callFuncBlockWithCache);

SORBET_INLINE
VALUE sorbet_callFuncBlockWithCache_noBreak(struct FunctionInlineCache *cache, BlockFFIType blockImpl, int blkMinArgs,
int blkMaxArgs, VALUE closure) {
VALUE sorbet_callFuncBlockWithCache_noBreak(struct FunctionInlineCache *cache, const struct vm_ifunc *ifunc) {
rb_execution_context_t *ec = GET_EC();
rb_control_frame_t *cfp = ec->cfp;

// This is an inlined version of the block handler setup that `rb_iterate` performs. See the following two links for
// the use of `rb_vm_ifunc_proc_new` and the setup of the captured block handler.
// * https://github.com/ruby/ruby/blob/ruby_2_7/vm_eval.c#L1448
// * https://github.com/ruby/ruby/blob/ruby_2_7/vm_eval.c#L1406-L1408
const struct vm_ifunc *const ifunc = rb_vm_ifunc_new(blockImpl, (void *)closure, blkMinArgs, blkMaxArgs);
struct rb_captured_block *captured = (struct rb_captured_block *)&cfp->self;
captured->code.ifunc = ifunc;

Expand Down Expand Up @@ -2365,8 +2367,8 @@ VALUE sorbet_inlineIntrinsicEnv_apply(VALUE value, BlockConsumerFFIType intrinsi
}

SORBET_INLINE
VALUE sorbet_callIntrinsicInlineBlock(VALUE (*body)(VALUE), VALUE recv, ID fun, int argc, VALUE *argv, BlockFFIType blk,
int blkMinArgs, int blkMaxArgs, VALUE closure) {
VALUE sorbet_callIntrinsicInlineBlock(VALUE (*body)(VALUE), VALUE recv, ID fun, int argc, VALUE *argv,
const struct vm_ifunc *ifunc, VALUE closure) {
struct sorbet_inlineIntrinsicEnv env;
env.recv = recv;
env.fun = fun;
Expand All @@ -2377,12 +2379,12 @@ VALUE sorbet_callIntrinsicInlineBlock(VALUE (*body)(VALUE), VALUE recv, ID fun,
// NOTE: we pass the block function to rb_iterate so that we ensure that the block handler is setup correctly.
// However it won't be called through the vm, as that would hide the direct call to the block function from the
// inliner.
return sorbet_rb_iterate(body, (VALUE)&env, blk, blkMinArgs, blkMaxArgs, closure);
return sorbet_rb_iterate(body, (VALUE)&env, ifunc);
}

SORBET_INLINE
VALUE sorbet_callIntrinsicInlineBlock_noBreak(VALUE (*body)(VALUE), VALUE recv, ID fun, int argc, VALUE *argv,
BlockFFIType blk, int blkMinArgs, int blkMaxArgs, VALUE closure) {
const struct vm_ifunc *ifunc, VALUE closure) {
struct sorbet_inlineIntrinsicEnv env;
env.recv = recv;
env.fun = fun;
Expand All @@ -2398,7 +2400,6 @@ VALUE sorbet_callIntrinsicInlineBlock_noBreak(VALUE (*body)(VALUE), VALUE recv,
rb_execution_context_t *ec = GET_EC();
rb_control_frame_t *cfp = ec->cfp;

const struct vm_ifunc *const ifunc = rb_vm_ifunc_new(blk, (void *)closure, blkMinArgs, blkMaxArgs);
struct rb_captured_block *captured = (struct rb_captured_block *)&cfp->self;
captured->code.ifunc = ifunc;
VALUE blockHandler = VM_BH_FROM_IFUNC_BLOCK(captured);
Expand Down Expand Up @@ -2539,7 +2540,7 @@ static VALUE sorbet_iterSuper(VALUE obj) {

SORBET_INLINE
VALUE sorbet_callSuperBlock(int argc, SORBET_ATTRIBUTE(noescape) const VALUE *const restrict argv, int kw_splat,
BlockFFIType blockImpl, int blkMinArgs, int blkMaxArgs, VALUE closure) {
const struct vm_ifunc *ifunc) {
// Mostly an implementation of return rb_call_super(argc, argv);
rb_execution_context_t *ec = GET_EC();
VALUE recv = ec->cfp->self;
Expand Down Expand Up @@ -2568,7 +2569,7 @@ VALUE sorbet_callSuperBlock(int argc, SORBET_ATTRIBUTE(noescape) const VALUE *co
arg.me = me;
arg.kw_splat = kw_splat;

return sorbet_rb_iterate(sorbet_iterSuper, (VALUE)&arg, blockImpl, blkMinArgs, blkMaxArgs, closure);
return sorbet_rb_iterate(sorbet_iterSuper, (VALUE)&arg, ifunc);
}

SORBET_INLINE
Expand Down
5 changes: 1 addition & 4 deletions compiler/IREmitter/Payload/patches/vm_insnhelper.c
View file
Open in desktop
Expand Up @@ -771,16 +771,13 @@ VALUE sorbet_vm_callBlock(rb_control_frame_t *cfp, int argc, const VALUE *const
}

// This is a version of rb_iterate specialized to the case where we know the block is non-null and its arity.
VALUE sorbet_rb_iterate(VALUE (*it_proc)(VALUE), VALUE data1, rb_block_call_func_t bl_proc, int minArgs, int maxArgs,
VALUE data2) {
VALUE sorbet_rb_iterate(VALUE (*it_proc)(VALUE), VALUE data1, const struct vm_ifunc *ifunc) {
rb_execution_context_t *ec = GET_EC();
rb_control_frame_t *const cfp = ec->cfp;

enum ruby_tag_type state;
volatile VALUE retval = Qnil;

const struct vm_ifunc *const ifunc = rb_vm_ifunc_new(bl_proc, (void *)data2, minArgs, maxArgs);

EC_PUSH_TAG(ec);
state = EC_EXEC_TAG();
if (state == 0) {
Expand Down
4 changes: 4 additions & 0 deletions compiler/IREmitter/Payload/vm-payload.c
View file
Open in desktop
Expand Up @@ -98,6 +98,10 @@ VALUE sorbet_globalConstDupHash(long idx) {
return rb_hash_dup(hash);
}

struct vm_ifunc *sorbet_globalConstFetchIfunc(long idx) {
return (struct vm_ifunc *)sorbet_globalConstFetch(idx);
}

// ****
// **** Calls
// ****
Expand Down
11 changes: 5 additions & 6 deletions compiler/IREmitter/SymbolBasedIntrinsics.cc
View file
Open in desktop
Expand Up @@ -185,16 +185,15 @@ class CallCMethod : public SymbolBasedIntrinsicMethod {
}
auto *forwarder = generateForwarder(mcctx);

auto arity = mcctx.irctx.rubyBlockArity[mcctx.blk.value()];
auto *minArgs = IREmitterHelpers::buildS4(cs, arity.min);
auto *maxArgs = IREmitterHelpers::buildS4(cs, arity.max);
auto blkId = mcctx.blk.value();

// NOTE: The ruby stack doesn't need to be managed here because the known c intrinsics don't expect to be
// called by the vm.
bool usesBreak = mcctx.irctx.blockUsesBreak[mcctx.blk.value()];
bool usesBreak = mcctx.irctx.blockUsesBreak[blkId];
auto *blkIfunc = Payload::buildBlockIfunc(cs, builder, mcctx.irctx, blkId);
if (usesBreak) {
res = builder.CreateCall(cs.module->getFunction("sorbet_callIntrinsicInlineBlock"),
{forwarder, recv, id, args.argc, args.argv, blk, minArgs, maxArgs, offset},
{forwarder, recv, id, args.argc, args.argv, blkIfunc, offset},
"rawSendResultWithBlock");
} else {
// Since the block doesn't use break we can make two optimizations:
Expand All @@ -204,7 +203,7 @@ class CallCMethod : public SymbolBasedIntrinsicMethod {
// 2. Emit a type assertion on the result of the function, as we know that there won't be non-local
// control flow based on the use of `break` that could change the type of the returned value
res = builder.CreateCall(cs.module->getFunction("sorbet_callIntrinsicInlineBlock_noBreak"),
{forwarder, recv, id, args.argc, args.argv, blk, minArgs, maxArgs, offset},
{forwarder, recv, id, args.argc, args.argv, blkIfunc, offset},
"rawSendResultWithBlock");
cMethodWithBlock->assertResultType(cs, builder, res);
}
Expand Down
19 changes: 7 additions & 12 deletions compiler/IREmitter/sends.cc
View file
Open in desktop
Expand Up @@ -434,9 +434,10 @@ llvm::Value *IREmitterHelpers::emitMethodCallViaRubyVM(MethodCallContext &mcctx)
auto arity = irctx.rubyBlockArity[mcctx.blk.value()];
auto *blkMinArgs = IREmitterHelpers::buildS4(cs, arity.min);
auto *blkMaxArgs = IREmitterHelpers::buildS4(cs, arity.max);
auto *ifunc = builder.CreateCall(cs.getFunction("sorbet_buildBlockIfunc"),
{blk, blkMinArgs, blkMaxArgs, localsOffset});
return builder.CreateCall(cs.getFunction("sorbet_callSuperBlock"),
{args.argc, args.argv, args.kw_splat, blk, blkMinArgs, blkMaxArgs, localsOffset},
"rawSendResult");
{args.argc, args.argv, args.kw_splat, ifunc}, "rawSendResult");
}

return builder.CreateCall(cs.getFunction("sorbet_callSuper"), {args.argc, args.argv, args.kw_splat},
Expand Down Expand Up @@ -523,25 +524,19 @@ llvm::Value *IREmitterHelpers::callViaRubyVMSimple(MethodCallContext &mcctx) {
auto *cache = mcctx.getInlineCache();
auto *recv = mcctx.varGetRecv();

auto *closure = Payload::buildLocalsOffset(mcctx.cs);
vector<llvm::Value *> args;
args.emplace_back(cache);

if (auto *blk = mcctx.blkAsFunction()) {
auto blkId = mcctx.blk.value();
args.emplace_back(llvm::ConstantInt::get(cs, llvm::APInt(1, static_cast<bool>(irctx.blockUsesBreak[blkId]))));
args.emplace_back(blk);

auto &arity = irctx.rubyBlockArity[mcctx.blk.value()];
args.emplace_back(IREmitterHelpers::buildS4(cs, arity.min));
args.emplace_back(IREmitterHelpers::buildS4(cs, arity.max));
auto *blkIfunc = Payload::buildBlockIfunc(cs, builder, irctx, blkId);
args.emplace_back(blkIfunc);
} else {
args.emplace_back(llvm::ConstantInt::get(cs, llvm::APInt(1, static_cast<bool>(false))));
args.emplace_back(llvm::ConstantPointerNull::get(llvm::PointerType::getUnqual(cs.getRubyBlockFFIType())));
args.emplace_back(IREmitterHelpers::buildS4(cs, 0));
args.emplace_back(IREmitterHelpers::buildS4(cs, 0));
auto *vmIfuncType = llvm::StructType::getTypeByName(cs, "struct.vm_ifunc");
args.emplace_back(llvm::ConstantPointerNull::get(llvm::PointerType::getUnqual(vmIfuncType)));
}
args.emplace_back(closure);
args.emplace_back(cfp);
args.emplace_back(recv);
for (auto *arg : stack) {
Expand Down