Skip to content

Commit

Permalink
Merge pull request #17504 from JuliaLang/yyc/codegen/plt
Browse files Browse the repository at this point in the history
Use `musttail` only as an optimization in the PLT.
  • Loading branch information
yuyichao committed Jul 20, 2016
2 parents 8091e05 + 84d5d6b commit ee5e231
Showing 1 changed file with 10 additions and 8 deletions.
18 changes: 10 additions & 8 deletions src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,15 @@ static DenseMap<AttributeSet,
std::map<std::tuple<GlobalVariable*,FunctionType*,
CallingConv::ID>,GlobalVariable*>> allPltMap;

#ifdef LLVM37 // needed for musttail
// Emit a "PLT" entry that will be lazily initialized
// when being called the first time.
static Value *emit_plt(FunctionType *functype, const AttributeSet &attrs,
CallingConv::ID cc, const char *f_lib, const char *f_name)
{
assert(imaging_mode);
// Don't do this for vararg functions so that the `musttail` is only
// an optimization and is not required to function correctly.
assert(!functype->isVarArg());
GlobalVariable *libptrgv;
GlobalVariable *llvmgv;
void *symaddr;
Expand Down Expand Up @@ -261,7 +263,12 @@ static Value *emit_plt(FunctionType *functype, const AttributeSet &attrs,
builder.CreateUnreachable();
}
else {
// musttail support is very bad on ARM, PPC, PPC64 (as of LLVM 3.9)
// Known failures includes vararg (not needed here) and sret.
#if defined(LLVM37) && (defined(_CPU_X86_) || defined(_CPU_X86_64_) || \
defined(_CPU_AARCH64_))
ret->setTailCallKind(CallInst::TCK_MustTail);
#endif
if (functype->getReturnType() == T_void) {
builder.CreateRetVoid();
}
Expand All @@ -287,7 +294,6 @@ static Value *emit_plt(FunctionType *functype, const AttributeSet &attrs,
}
return builder.CreateBitCast(builder.CreateLoad(got), funcptype);
}
#endif

// --- ABI Implementations ---
// Partially based on the LDC ABI implementations licensed under the BSD 3-clause license
Expand Down Expand Up @@ -1703,16 +1709,12 @@ static jl_cgval_t emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)

PointerType *funcptype = PointerType::get(functype,0);
if (imaging_mode) {
#ifdef LLVM37
// ARM, PPC, PPC64 (as of LLVM 3.9) don't support `musttail` for vararg functions.
// And musttail can't precede unreachable, but is required for vararg (https://llvm.org/bugs/show_bug.cgi?id=23766)
// vararg requires musttail,
// but musttail is incompatible with noreturn.
if (functype->isVarArg())
llvmf = runtime_sym_lookup(funcptype, f_lib, f_name, ctx->f);
else
llvmf = emit_plt(functype, attrs, cc, f_lib, f_name);
#else
llvmf = runtime_sym_lookup(funcptype, f_lib, f_name, ctx->f);
#endif
}
else {
void *symaddr = jl_dlsym_e(jl_get_library(f_lib), f_name);
Expand Down

3 comments on commit ee5e231

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@jrevels
Copy link
Member

Choose a reason for hiding this comment

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

To any onlookers curious about these extreme changes: the comparison here is not actually against the previous day's build. Yesterday, an extra daily build was accidentally triggered from a PR (the comment which triggered it has since been deleted, it seems), overwriting the actual daily build. We should add a check in Nanosoldier.jl that disallows daily build submissions from PRs in the future (maybe even disallow daily build submissions from anybody besides @nanosoldier).

The right thing to do now is probably just to revert the commit which added the erroneous daily build in the report repository, restoring the correct daily build results. Then, we could just regenerate today's report (note that today's data is correct, and doesn't need to be regathered; only the report needs to be regenerated). I might not have time to take care of this today, but I'll put it on my to-do list.

Please sign in to comment.