From 081efcbb7dc265a304a8d2ea8ee229d7a7a4fe6a Mon Sep 17 00:00:00 2001 From: Frits van Bommel Date: Sat, 18 Apr 2009 00:34:20 +0200 Subject: [PATCH 1/4] Copy alloca'd parameters referenced by nested functions to the nesting frame. --- gen/nested.cpp | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/gen/nested.cpp b/gen/nested.cpp index a0e2ff6d..aab892fd 100644 --- a/gen/nested.cpp +++ b/gen/nested.cpp @@ -426,10 +426,14 @@ void DtoCreateNestedContext(FuncDeclaration* fd) { vd->ir.irLocal->nestedDepth = depth; if (vd->isParameter()) { // Parameters already have storage associated with them (to handle byref etc.), - // so handle specially for now by storing a pointer instead of a value. + // so handle those cases specially by storing a pointer instead of a value. assert(vd->ir.irLocal->value); - // FIXME: don't do this for normal parameters? - types.push_back(vd->ir.irLocal->value->getType()); + LLValue* value = vd->ir.irLocal->value; + const LLType* type = value->getType(); + if (llvm::isa(value->getUnderlyingObject())) + // This will be copied to the nesting frame. + type = type->getContainedType(0); + types.push_back(type); } else if (vd->isRef() || vd->isOut()) { // Foreach variables can also be by reference, for instance. types.push_back(DtoType(vd->type->pointerTo())); @@ -453,6 +457,8 @@ void DtoCreateNestedContext(FuncDeclaration* fd) { // Create frame for current function and append to frames list // FIXME: For D2, this should be a gc_malloc (or similar) call, not alloca + // (Note that it'd also require more aggressive copying of + // by-value parameters instead of just alloca'd ones) LLValue* frame = DtoAlloca(frameType, ".frame"); // copy parent frames into beginning @@ -491,8 +497,26 @@ void DtoCreateNestedContext(FuncDeclaration* fd) { LLValue* gep = DtoGEPi(frame, 0, vd->ir.irLocal->nestedIndex, vd->toChars()); if (vd->isParameter()) { Logger::println("nested param: %s", vd->toChars()); - DtoAlignedStore(vd->ir.irLocal->value, gep); - vd->ir.irLocal->byref = true; + LOG_SCOPE + LLValue* value = vd->ir.irLocal->value; + if (llvm::isa(value->getUnderlyingObject())) { + Logger::println("Copying to nested frame"); + // The parameter value is an alloca'd stack slot. + // Copy to the nesting frame and leave the alloca for + // the optimizers to clean up. + DtoStore(DtoLoad(value), gep); + gep->takeName(value); + vd->ir.irLocal->value = gep; + vd->ir.irLocal->byref = false; + } else { + Logger::println("Adding pointer to nested frame"); + // The parameter value is something else, such as a + // passed-in pointer (for 'ref' or 'out' parameters) or + // a pointer arg with byval attribute. + // Store the address into the frame and set the byref flag. + DtoAlignedStore(vd->ir.irLocal->value, gep); + vd->ir.irLocal->byref = true; + } } else if (vd->isRef() || vd->isOut()) { // This slot is initialized in DtoNestedInit, to handle things like byref foreach variables // which move around in memory. From e05b960bbe9a28c70bc83b0f6b5cb2ed9bc782fc Mon Sep 17 00:00:00 2001 From: Frits van Bommel Date: Sun, 19 Apr 2009 19:28:10 +0200 Subject: [PATCH 2/4] No need for temporary alloca's here, use a phi node instead. --- gen/toir.cpp | 53 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/gen/toir.cpp b/gen/toir.cpp index e5d2e98e..0e4e2fa4 100644 --- a/gen/toir.cpp +++ b/gen/toir.cpp @@ -1828,10 +1828,6 @@ DValue* AndAndExp::toElem(IRState* p) Logger::print("AndAndExp::toElem: %s @ %s\n", toChars(), type->toChars()); LOG_SCOPE; - // allocate a temporary for the final result. failed to come up with a better way :/ - LLValue* resval = 0; - resval = DtoAlloca(LLType::Int1Ty,"andandtmp"); - DValue* u = e1->toElem(p); llvm::BasicBlock* oldend = p->scopeend(); @@ -1839,23 +1835,36 @@ DValue* AndAndExp::toElem(IRState* p) llvm::BasicBlock* andandend = llvm::BasicBlock::Create("andandend", gIR->topfunc(), oldend); LLValue* ubool = DtoCast(loc, u, Type::tbool)->getRVal(); - DtoStore(ubool,resval); + + llvm::BasicBlock* oldblock = p->scopebb(); llvm::BranchInst::Create(andand,andandend,ubool,p->scopebb()); p->scope() = IRScope(andand, andandend); DValue* v = e2->toElem(p); + LLValue* vbool = 0; if (!v->isFunc() && v->getType() != Type::tvoid) { - LLValue* vbool = DtoCast(loc, v, Type::tbool)->getRVal(); - LLValue* uandvbool = llvm::BinaryOperator::Create(llvm::BinaryOperator::And, ubool, vbool,"tmp",p->scopebb()); - DtoStore(uandvbool,resval); + vbool = DtoCast(loc, v, Type::tbool)->getRVal(); } + llvm::BasicBlock* newblock = p->scopebb(); llvm::BranchInst::Create(andandend,p->scopebb()); p->scope() = IRScope(andandend, oldend); - resval = DtoLoad(resval); + LLValue* resval = 0; + if (ubool == vbool || !vbool) { + // No need to create a PHI node. + resval = ubool; + } else { + llvm::PHINode* phi = p->ir->CreatePHI(LLType::Int1Ty, "andandval"); + // If we jumped over evaluation of the right-hand side, + // the result is false. Otherwise it's the value of the right-hand side. + phi->addIncoming(LLConstantInt::getFalse(), oldblock); + phi->addIncoming(vbool, newblock); + resval = phi; + } + return new DImValue(type, resval); } @@ -1866,10 +1875,6 @@ DValue* OrOrExp::toElem(IRState* p) Logger::print("OrOrExp::toElem: %s @ %s\n", toChars(), type->toChars()); LOG_SCOPE; - // allocate a temporary for the final result. failed to come up with a better way :/ - LLValue* resval = 0; - resval = DtoAlloca(LLType::Int1Ty,"orortmp"); - DValue* u = e1->toElem(p); llvm::BasicBlock* oldend = p->scopeend(); @@ -1877,22 +1882,36 @@ DValue* OrOrExp::toElem(IRState* p) llvm::BasicBlock* ororend = llvm::BasicBlock::Create("ororend", gIR->topfunc(), oldend); LLValue* ubool = DtoCast(loc, u, Type::tbool)->getRVal(); - DtoStore(ubool,resval); + + llvm::BasicBlock* oldblock = p->scopebb(); llvm::BranchInst::Create(ororend,oror,ubool,p->scopebb()); p->scope() = IRScope(oror, ororend); DValue* v = e2->toElem(p); + LLValue* vbool = 0; if (!v->isFunc() && v->getType() != Type::tvoid) { - LLValue* vbool = DtoCast(loc, v, Type::tbool)->getRVal(); - DtoStore(vbool,resval); + vbool = DtoCast(loc, v, Type::tbool)->getRVal(); } + llvm::BasicBlock* newblock = p->scopebb(); llvm::BranchInst::Create(ororend,p->scopebb()); p->scope() = IRScope(ororend, oldend); - resval = DtoLoad(resval); + LLValue* resval = 0; + if (ubool == vbool || !vbool) { + // No need to create a PHI node. + resval = ubool; + } else { + llvm::PHINode* phi = p->ir->CreatePHI(LLType::Int1Ty, "ororval"); + // If we jumped over evaluation of the right-hand side, + // the result is true. Otherwise, it's the value of the right-hand side. + phi->addIncoming(LLConstantInt::getTrue(), oldblock); + phi->addIncoming(vbool, newblock); + resval = phi; + } + return new DImValue(type, resval); } From 21f273671a1bc58d6dbe95b81513f0626fe947df Mon Sep 17 00:00:00 2001 From: Frits van Bommel Date: Sun, 19 Apr 2009 23:15:03 +0200 Subject: [PATCH 3/4] Fix a problem which occurred when a function type was forward-referenced by parameter types. This was intended to fix the following test case: {{{ void delegate(Hit) dg; struct Hit { void delegate(Hit) a; } }}} which ChristianK reduced from a problem downs had on IRC. It also seems to fix mini/compile_delegate.d (which turns out to be quite similar, but produced a different error message). --- gen/functions.cpp | 53 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/gen/functions.cpp b/gen/functions.cpp index 212c1055..df639db6 100644 --- a/gen/functions.cpp +++ b/gen/functions.cpp @@ -45,8 +45,9 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest gABI->newFunctionType(f); } - // start new ir funcTy - f->fty.reset(); + // Do not modify f->fty yet; this function may be called recursively if any + // of the argument types refer to this type. + IrFuncTy fty; // llvm idx counter size_t lidx = 0; @@ -54,7 +55,7 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest // main needs a little special handling if (ismain) { - f->fty.ret = new IrFuncTyArg(Type::tint32, false); + fty.ret = new IrFuncTyArg(Type::tint32, false); } // sane return value else @@ -65,7 +66,7 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest if (f->linkage != LINKintrinsic) if (gABI->returnInArg(f)) { - f->fty.arg_sret = new IrFuncTyArg(rt, true, StructRet | NoAlias | NoCapture); + fty.arg_sret = new IrFuncTyArg(rt, true, StructRet | NoAlias | NoCapture); rt = Type::tvoid; lidx++; } @@ -74,7 +75,7 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest { a = se; } - f->fty.ret = new IrFuncTyArg(rt, false, a); + fty.ret = new IrFuncTyArg(rt, false, a); } lidx++; @@ -82,14 +83,14 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest if (thistype) { bool toref = (thistype->toBasetype()->ty == Tstruct); - f->fty.arg_this = new IrFuncTyArg(thistype, toref); + fty.arg_this = new IrFuncTyArg(thistype, toref); lidx++; } // and nested functions else if (nesttype) { - f->fty.arg_nest = new IrFuncTyArg(nesttype, false); + fty.arg_nest = new IrFuncTyArg(nesttype, false); lidx++; } @@ -103,16 +104,16 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest if (f->varargs == 1) { // _arguments - f->fty.arg_arguments = new IrFuncTyArg(Type::typeinfo->type->arrayOf(), false); + fty.arg_arguments = new IrFuncTyArg(Type::typeinfo->type->arrayOf(), false); lidx++; // _argptr - f->fty.arg_argptr = new IrFuncTyArg(Type::tvoid->pointerTo(), false, NoAlias | NoCapture); + fty.arg_argptr = new IrFuncTyArg(Type::tvoid->pointerTo(), false, NoAlias | NoCapture); lidx++; } } else if (f->linkage == LINKc) { - f->fty.c_vararg = true; + fty.c_vararg = true; } else { @@ -127,7 +128,7 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest if (ismain && nargs == 0) { Type* mainargs = Type::tchar->arrayOf()->arrayOf(); - f->fty.args.push_back(new IrFuncTyArg(mainargs, false)); + fty.args.push_back(new IrFuncTyArg(mainargs, false)); lidx++; } // add explicit parameters @@ -163,10 +164,38 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest a |= DtoShouldExtend(argtype); } - f->fty.args.push_back(new IrFuncTyArg(argtype, byref, a)); + fty.args.push_back(new IrFuncTyArg(argtype, byref, a)); lidx++; } + // If the function type was forward referenced by one of the parameter types, + // it has now been set. + if (f->ir.type) { + for (size_t i = 0; i < fty.args.size(); i++) { + Logger::cout() << "Arg type: " << *fty.args[i]->ltype << '\n'; + } + + // Notify ABI that we won't be needing it for this function type anymore. + gABI->doneWithFunctionType(); + + // Some cleanup of memory we won't use + delete fty.ret; + delete fty.arg_sret; + delete fty.arg_this; + delete fty.arg_nest; + delete fty.arg_arguments; + delete fty.arg_argptr; + for (IrFuncTy::ArgIter It = fty.args.begin(), E = fty.args.end(); It != E; ++It) { + delete *It; + } + + Logger::cout() << "Function type: " << **f->ir.type << '\n'; + return llvm::cast(*f->ir.type); + } + + // Now we can modify f->fty safely. + f->fty = fty; + if (f->linkage != LINKintrinsic) { // let the abi rewrite the types as necesary gABI->rewriteFunctionType(f); From 0e8216fbd614b291db65ce9a8c9329199f14e0f3 Mon Sep 17 00:00:00 2001 From: Frits van Bommel Date: Mon, 20 Apr 2009 00:04:35 +0200 Subject: [PATCH 4/4] Remove some logging I didn't mean to commit. --- gen/functions.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/gen/functions.cpp b/gen/functions.cpp index df639db6..b4bccac3 100644 --- a/gen/functions.cpp +++ b/gen/functions.cpp @@ -171,10 +171,6 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest // If the function type was forward referenced by one of the parameter types, // it has now been set. if (f->ir.type) { - for (size_t i = 0; i < fty.args.size(); i++) { - Logger::cout() << "Arg type: " << *fty.args[i]->ltype << '\n'; - } - // Notify ABI that we won't be needing it for this function type anymore. gABI->doneWithFunctionType(); @@ -189,7 +185,7 @@ const llvm::FunctionType* DtoFunctionType(Type* type, Type* thistype, Type* nest delete *It; } - Logger::cout() << "Function type: " << **f->ir.type << '\n'; + Logger::cout() << "Final function type: " << **f->ir.type << '\n'; return llvm::cast(*f->ir.type); }