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

Add pragma(musttail) #4366

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions dmd/expression.d
Original file line number Diff line number Diff line change
Expand Up @@ -5077,6 +5077,10 @@ extern (C++) final class CallExp : UnaExp
bool directcall; // true if a virtual call is devirtualized
bool inDebugStatement; /// true if this was in a debug statement
bool ignoreAttributes; /// don't enforce attributes (e.g. call @gc function in @nogc code)
version (IN_LLVM)
{
bool isMustTail; // If marked with pragma(musttail)
}
VarDeclaration vthis2; // container for multi-context

extern (D) this(const ref Loc loc, Expression e, Expressions* exps)
Expand Down
3 changes: 3 additions & 0 deletions dmd/expression.h
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,9 @@ class CallExp final : public UnaExp
bool directcall; // true if a virtual call is devirtualized
bool inDebugStatement; // true if this was in a debug statement
bool ignoreAttributes; // don't enforce attributes (e.g. call @gc function in @nogc code)
#if IN_LLVM
bool isMustTail; // If marked with pragma(musttail)
#endif
VarDeclaration *vthis2; // container for multi-context

static CallExp *create(const Loc &loc, Expression *e, Expressions *exps);
Expand Down
1 change: 1 addition & 0 deletions dmd/id.d
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ immutable Msgtable[] msgtable =
{ "LDC_global_crt_dtor" },
{ "LDC_extern_weak" },
{ "LDC_profile_instr" },
{ "musttail" },

// IN_LLVM: LDC-specific traits
{ "targetCPU" },
Expand Down
1 change: 1 addition & 0 deletions dmd/id.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ struct Id
static Identifier *LDC_inline_ir;
static Identifier *LDC_extern_weak;
static Identifier *LDC_profile_instr;
static Identifier *musttail;
static Identifier *dcReflect;
static Identifier *opencl;
static Identifier *criticalenter;
Expand Down
39 changes: 39 additions & 0 deletions dmd/statementsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -2135,6 +2135,13 @@ else
return setError();
}
}
else if (ps.ident == Id.musttail)
{
version (IN_LLVM)
{
pragmaMustTailSemantic(ps);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This is a frontend file too (everything in dmd/), so IN_LLVM again. Note that there's a IN_LLVM global/enum somewhere too, so that it's easy to use in if expressions.

And version (IN_LLVM) for pragmaMustTailSemantic() below too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should non-ldc compilers do for this pragma? Will this file be syncronized with dmd at some point?

Copy link
Member

Choose a reason for hiding this comment

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

Upstreaming our frontend diff is probably not realistic; some of it are hacks, and some others really totally LDC-specific and would just clutter the DMD frontend IMO. If another compiler supported it, we'd upstream a proper semantic for the new pragma though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Clutter is meaningless versus the asymptotic cost of complexity - it should be upstreamed. LDC is the real D compiler, the codebase should be honest about that

Copy link
Contributor

Choose a reason for hiding this comment

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

Slightly less abstract: we should get a way upstream for there to no more IN_LLVM and so on other than registering a behaviour (and not "just override a weak symbol")

else if (!global.params.ignoreUnsupportedPragmas)
{
ps.error("unrecognized `pragma(%s)`", ps.ident.toChars());
Expand All @@ -2153,6 +2160,38 @@ else
result = ps._body;
}

version (IN_LLVM)
private void pragmaMustTailSemantic(PragmaStatement ps)
{
if (!ps._body)
{
ps.error("`pragma(musttail)` must be attached to a return statement");
return setError();
}

auto rs = ps._body.isReturnStatement();
if (!rs)
{
ps.error("`pragma(musttail)` must be attached to a return statement");
return setError();
}

if (!rs.exp)
{
ps.error("`pragma(musttail)` must be attached to a return statement returning result of a function call");
return setError();
}

auto ce = rs.exp.isCallExp();
if (!ce)
{
ps.error("`pragma(musttail)` must be attached to a return statement returning result of a function call");
return setError();
}

ce.isMustTail = true;
}

override void visit(StaticAssertStatement s)
{
s.sa.semantic2(sc);
Expand Down
3 changes: 2 additions & 1 deletion gen/dpragma.d
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ extern (C++) enum LDCPragma : int {
LLVMbitop_bts,
LLVMbitop_vld,
LLVMbitop_vst,
LLVMextern_weak
LLVMextern_weak,
LLVMmusttail,
};

extern (C++) LDCPragma DtoGetPragma(Scope* sc, PragmaDeclaration decl, ref const(char)* arg1str);
Expand Down
3 changes: 2 additions & 1 deletion gen/llvmhelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ bool DtoLowerMagicIntrinsic(IRState *p, FuncDeclaration *fndecl, CallExp *e,

///
DValue *DtoCallFunction(const Loc &loc, Type *resulttype, DValue *fnval,
Expressions *arguments, LLValue *sretPointer = nullptr);
Expressions *arguments, LLValue *sretPointer = nullptr,
bool isMustTail = false);

Type *stripModifiers(Type *type, bool transitive = false);

Expand Down
9 changes: 9 additions & 0 deletions gen/pragma.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,15 @@ LDCPragma DtoGetPragma(Scope *sc, PragmaDeclaration *decl,
return LLVMprofile_instr;
}

// pragma(musttail)
if (ident == Id::musttail) {
if (args && args->length > 0) {
decl->error("takes no parameters");
fatal();
}
return LLVMmusttail;
}

return LLVMnone;
}

Expand Down
3 changes: 2 additions & 1 deletion gen/pragma.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ enum LDCPragma {
LLVMbitop_vld,
LLVMbitop_vst,
LLVMextern_weak,
LLVMprofile_instr
LLVMprofile_instr,
LLVMmusttail,
};

LDCPragma DtoGetPragma(Scope *sc, PragmaDeclaration *decl, const char *&arg1str);
Expand Down
15 changes: 14 additions & 1 deletion gen/tocall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,8 @@ static LLValue *DtoCallableValue(llvm::FunctionType * ft,DValue *fn) {

// FIXME: this function is a mess !
DValue *DtoCallFunction(const Loc &loc, Type *resulttype, DValue *fnval,
Expressions *arguments, LLValue *sretPointer) {
Expressions *arguments, LLValue *sretPointer,
bool isMustTail) {
IF_LOG Logger::println("DtoCallFunction()");
LOG_SCOPE

Expand Down Expand Up @@ -1065,6 +1066,18 @@ DValue *DtoCallFunction(const Loc &loc, Type *resulttype, DValue *fnval,
llvm::AttrBuilder(call->getAttributes(), LLAttributeList::FunctionIndex));
#endif
call->setAttributes(attrlist);
if (isMustTail) {
if (auto ci = llvm::dyn_cast<llvm::CallInst>(call)) {
ci->setTailCallKind(llvm::CallInst::TCK_MustTail);
} else {
if (!tf->isnothrow()) {
error(loc, "cannot perform tail-call - callee must be nothrow");
Copy link
Member

Choose a reason for hiding this comment

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

can this error not be caught during semantic analysis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where in semantic can I check this? I'm not really familiar with the code.

} else {
error(loc, "cannot perform tail-call - no code like destructors or scope(exit) should run after the call");
Copy link
Member

Choose a reason for hiding this comment

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

same as above, can this error not be caught during semantic analysis?

}
fatal();
}
}

// Special case for struct constructor calls: For temporaries, using the
// this pointer value returned from the constructor instead of the alloca
Expand Down
2 changes: 1 addition & 1 deletion gen/toir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ class ToElemVisitor : public Visitor {
}

DValue *result =
DtoCallFunction(e->loc, e->type, fnval, e->arguments, sretPointer);
DtoCallFunction(e->loc, e->type, fnval, e->arguments, sretPointer, e->isMustTail);

if (delayedDtorVar) {
delayedDtorVar->edtor = delayedDtorExp;
Expand Down
14 changes: 14 additions & 0 deletions tests/codegen/musttail_1.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Tests successfull musttail application

// RUN: %ldc -output-ll -of=%t.ll %s && FileCheck %s < %t.ll

// CHECK-LABEL: define{{.*}} @{{.*}}foo
int foo(int x) nothrow
{
// CHECK: musttail call{{.*}} @{{.*}}bar
// CHECK-NEXT: ret i32
pragma(musttail) return bar(x);
}

// CHECK-LABEL: define{{.*}} @{{.*}}bar
int bar(int x) nothrow { return x; }