diff --git a/gen/declarations.cpp b/gen/declarations.cpp index c3ceff4da15..da3bffa2964 100644 --- a/gen/declarations.cpp +++ b/gen/declarations.cpp @@ -206,10 +206,15 @@ class CodegenVisitor : public Visitor { } // Define the __initZ symbol. - IrAggr *ir = getIrAggr(decl); - llvm::GlobalVariable *initZ = ir->getInitSymbol(); - initZ->setInitializer(ir->getDefaultInit()); - setLinkage(decl, initZ); + // If we reach here during codegen of an available_externally function, + // the struct initializer should stay external and therefore must not + // have an initializer. + if ((decl->getModule() == gIR->dmodule) || decl->isInstantiated()) { + IrAggr *ir = getIrAggr(decl); + llvm::GlobalVariable *initZ = ir->getInitSymbol(); + initZ->setInitializer(ir->getDefaultInit()); + setLinkage(decl, initZ); + } // emit typeinfo DtoTypeInfoOf(decl->type); @@ -341,7 +346,8 @@ class CodegenVisitor : public Visitor { // If we reach here during codegen of an available_externally function, // new variable declarations should stay external and therefore must not // have an initializer. - if (!(decl->storage_class & STCextern) && !decl->inNonRoot()) { + if (!(decl->storage_class & STCextern) && + ((decl->getModule() == gIR->dmodule) || decl->isInstantiated())) { // Build the initializer. Might use this->ir.irGlobal->value! LLConstant *initVal = DtoConstInitializer(decl->loc, decl->type, decl->_init); diff --git a/gen/function-inlining.cpp b/gen/function-inlining.cpp index 37c64e13638..8005e98963f 100644 --- a/gen/function-inlining.cpp +++ b/gen/function-inlining.cpp @@ -143,18 +143,21 @@ bool defineAsExternallyAvailable(FuncDeclaration &fdecl) { return false; } - if (fdecl.semanticRun >= PASSsemantic3) { + if ((fdecl.inlining != PINLINEalways) && fdecl.semanticRun >= PASSsemantic3) { // If semantic analysis has come this far, the function will be defined // elsewhere and should not get the available_externally attribute from - // here. + // here. However, pragma(inline, true) functions should remain + // defineAsExternallyAvailable candidates to fix GH #1712. // TODO: This check prevents inlining of nested functions. IF_LOG Logger::println("Semantic analysis already completed"); return false; } - if (alreadyOrWillBeDefined(fdecl)) { + if ((fdecl.inlining != PINLINEalways) && alreadyOrWillBeDefined(fdecl)) { // This check is needed because of ICEs happening because of unclear issues // upon changing the codegen order without this check. + // pragma(inline, true) functions remain defineAsExternallyAvailable + // candidates to fix GH #1712. IF_LOG Logger::println("Function will be defined later."); return false; } @@ -170,6 +173,7 @@ bool defineAsExternallyAvailable(FuncDeclaration &fdecl) { IF_LOG Logger::println("Potential inlining candidate"); + if (fdecl.semanticRun < PASSsemantic3) { IF_LOG Logger::println("Do semantic analysis"); LOG_SCOPE diff --git a/gen/functions.cpp b/gen/functions.cpp index 358631f851f..f575bd68469 100644 --- a/gen/functions.cpp +++ b/gen/functions.cpp @@ -325,7 +325,9 @@ static llvm::Function *DtoDeclareVaFunction(FuncDeclaration *fdecl) { //////////////////////////////////////////////////////////////////////////////// -void DtoResolveFunction(FuncDeclaration *fdecl) { +/// When mayDefine is true, the call to this function may possibly define the +/// function too (e.g. for cross-module inlining). +void DtoResolveFunction(FuncDeclaration *fdecl, bool mayDefine) { if ((!global.params.useUnitTests || !fdecl->type) && fdecl->isUnitTestDeclaration()) { IF_LOG Logger::println("Ignoring unittest %s", fdecl->toPrettyChars()); @@ -404,7 +406,7 @@ void DtoResolveFunction(FuncDeclaration *fdecl) { // queue declaration unless the function is abstract without body if (!fdecl->isAbstract() || fdecl->fbody) { - DtoDeclareFunction(fdecl); + DtoDeclareFunction(fdecl, mayDefine); } } @@ -434,8 +436,10 @@ void applyDefaultMathAttributes(IrFunction *irFunc) { //////////////////////////////////////////////////////////////////////////////// -void DtoDeclareFunction(FuncDeclaration *fdecl) { - DtoResolveFunction(fdecl); +/// When mayDefine is true, the call to this function may possibly define the +/// function too (e.g. for cross-module inlining). +void DtoDeclareFunction(FuncDeclaration *fdecl, bool mayDefine) { + DtoResolveFunction(fdecl, mayDefine); if (fdecl->ir->isDeclared()) { return; @@ -462,7 +466,8 @@ void DtoDeclareFunction(FuncDeclaration *fdecl) { // Check if fdecl should be defined too for cross-module inlining. // If true, semantic is fully done for fdecl which is needed for some code // below (e.g. code that uses fdecl->vthis). - const bool defineAtEnd = defineAsExternallyAvailable(*fdecl); + const bool defineAtEnd = + mayDefine && defineAsExternallyAvailable(*fdecl); if (defineAtEnd) { IF_LOG Logger::println( "Function is an externally_available inline candidate."); @@ -742,7 +747,16 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) { fd->loc.toChars()); LOG_SCOPE; if (linkageAvailableExternally) { - IF_LOG Logger::println("linkageAvailableExternally = true"); + IF_LOG Logger::println("linkageAvailableExternally == true"); + } + + if ((fd->getModule() != gIR->dmodule) && !fd->isInstantiated()) { + // The function is being emitted cross-module for inlining. We reach here + // for nested functions inside cross-module emitted functions. We have to + // explicitly set `linkageAvailableExternally` to true. + linkageAvailableExternally = true; + IF_LOG Logger::println("Cross-module emitted nested function: " + "linkageAvailableExternally = true"); } if (fd->ir->isDefined()) { @@ -750,6 +764,7 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) { (getIrFunc(fd)->func->getLinkage() == llvm::GlobalValue::AvailableExternallyLinkage)) { // Fix linkage + IF_LOG Logger::println("Remove available_externally linkage."); const auto lwc = lowerFuncLinkage(fd); setLinkage(lwc, getIrFunc(fd)->func); } @@ -777,7 +792,7 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) { fatal(); } - DtoResolveFunction(fd); + DtoResolveFunction(fd, false); if (fd->isUnitTestDeclaration() && !global.params.useUnitTests) { IF_LOG Logger::println("No code generation for unit test declaration %s", @@ -801,7 +816,7 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) { return; } - DtoDeclareFunction(fd); + DtoDeclareFunction(fd, false); assert(fd->ir->isDeclared()); // DtoResolveFunction might also set the defined flag for functions we diff --git a/gen/functions.h b/gen/functions.h index 0b44e9475f9..008e3b3b0d9 100644 --- a/gen/functions.h +++ b/gen/functions.h @@ -34,8 +34,8 @@ llvm::FunctionType *DtoFunctionType(Type *t, IrFuncTy &irFty, Type *thistype, bool hasSel = false); llvm::FunctionType *DtoFunctionType(FuncDeclaration *fdecl); -void DtoResolveFunction(FuncDeclaration *fdecl); -void DtoDeclareFunction(FuncDeclaration *fdecl); +void DtoResolveFunction(FuncDeclaration *fdecl, bool mayDefine = true); +void DtoDeclareFunction(FuncDeclaration *fdecl, bool mayDefine = true); void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally = false); void DtoDefineNakedFunction(FuncDeclaration *fd); diff --git a/tests/codegen/inlining_gh1712.d b/tests/codegen/inlining_gh1712.d new file mode 100644 index 00000000000..0180a77b743 --- /dev/null +++ b/tests/codegen/inlining_gh1712.d @@ -0,0 +1,46 @@ +// Test inlining related to Github issue 1712 + +// REQUIRES: atleast_llvm307 + +// RUN: %ldc %s -c -output-ll -O3 -of=%t.ll && FileCheck %s < %t.ll +// RUN: %ldc -O3 -run %s + +module mod; + +void test9785_2() +{ + int j = 3; + + void loop(scope const void function(int x) dg) + { + pragma(inline, true); + dg(++j); + } + + static void func(int x) + { + pragma(inline, true); + assert(x == 4); + } + + loop(&func); +} + +void main() +{ + test9785_2(); +} + +// There was a bug where pragma(inline, true) nested functions were incorrectly emitted with `available_externally` linkage. + +// CHECK: define +// CHECK-NOT: available_externally +// CHECK-SAME: @{{.*}}D3mod10test9785_2FZv + +// CHECK: define +// CHECK-NOT: available_externally +// CHECK-SAME: @{{.*}}D3mod10test9785_2FZ4loopMFMxPFiZvZv + +// CHECK: define +// CHECK-NOT: available_externally +// CHECK-SAME: @{{.*}}D3mod10test9785_2FZ4funcFiZv diff --git a/tests/codegen/inlining_gh1712_originalbug.d b/tests/codegen/inlining_gh1712_originalbug.d new file mode 100644 index 00000000000..1ac66448dfb --- /dev/null +++ b/tests/codegen/inlining_gh1712_originalbug.d @@ -0,0 +1,15 @@ +// REQUIRES: atleast_llvm307 + +// RUN: %ldc -output-ll -od=%T -I%S -O0 -release -enable-cross-module-inlining %s %S/inputs/inlining_gh1712_string.d \ +// RUN: && FileCheck %s < %T/inlining_gh1712_originalbug.ll + +import inputs.inlinables; + +void main() +{ + call_template_foo(1); +} + +// Make sure that the pragma(inline, true) function `call_template_foo` is defined, and not just declared. +// When it is correctly defined, optimization (available_externally+alwaysinline) even at -O0 means that it will dissappear from IR. +// CHECK-NOT: declare {{.*}} @call_template_foo \ No newline at end of file diff --git a/tests/codegen/inlining_staticvar.d b/tests/codegen/inlining_staticvar.d index e4b7356999f..971be92652c 100644 --- a/tests/codegen/inlining_staticvar.d +++ b/tests/codegen/inlining_staticvar.d @@ -4,6 +4,12 @@ // RUN: %ldc %s -I%S -c -output-ll -O3 -of=%t.O3.ll && FileCheck %s --check-prefix OPT3 < %t.O3.ll // RUN: %ldc %s -I%S -c -output-ll -enable-inlining -O0 -of=%t.O0.ll && FileCheck %s --check-prefix OPT0 < %t.O0.ll + +// Test linkage with separate compilation. +// RUN: %ldc -c %S/inputs/inlinables_staticvar.d -of=%t.import%obj \ +// RUN: && %ldc -I%S -enable-inlining %t.import%obj -run %s + +// Test linkage with non-singleobj build. // RUN: %ldc -I%S -enable-inlining %S/inputs/inlinables_staticvar.d -run %s // RUN: %ldc -I%S -O3 %S/inputs/inlinables_staticvar.d -run %s @@ -55,6 +61,16 @@ void checkNestedStruct_2() @weak assert(addAndCheckNestedStruct(7+101, 9)); } +void checkTemplatedNestedStruct_1() @weak +{ + assert(addAndCheckTemplatedNestedStruct(0, 7)); +} +void checkTemplatedNestedStruct_2() @weak +{ + assert(addAndCheckTemplatedNestedStructIndirect(7, 101)); + assert(addAndCheckTemplatedNestedStruct(7+101, 9)); +} + // OPT0-LABEL: define{{.*}} @_Dmain( // OPT3-LABEL: define{{.*}} @_Dmain( extern(D) @@ -68,4 +84,6 @@ void main() checkInsideNestedFunc_2(); checkNestedStruct_1(); checkNestedStruct_2(); + checkTemplatedNestedStruct_1(); + checkTemplatedNestedStruct_2(); } diff --git a/tests/codegen/inputs/inlinables_staticvar.d b/tests/codegen/inputs/inlinables_staticvar.d index fad14addff2..cfb2f2133d9 100644 --- a/tests/codegen/inputs/inlinables_staticvar.d +++ b/tests/codegen/inputs/inlinables_staticvar.d @@ -83,3 +83,24 @@ pragma(inline, false) bool addAndCheckNestedStructIndirect(int checkbefore, int } /+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++/ + +pragma(inline, true) bool addAndCheckTemplatedNestedStruct(T)(T checkbefore, T increment) +{ + struct NestedStruct(T) + { + static T structValue; + } + + if (NestedStruct!T.structValue != checkbefore) + return false; + + NestedStruct!T.structValue += increment; + return true; +} + +pragma(inline, false) bool addAndCheckTemplatedNestedStructIndirect(T)(T checkbefore, T increment) +{ + return addAndCheckTemplatedNestedStruct!T(checkbefore, increment); +} + +/+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++/ diff --git a/tests/codegen/inputs/inlining_gh1712_string.d b/tests/codegen/inputs/inlining_gh1712_string.d new file mode 100644 index 00000000000..4fb1dc2150b --- /dev/null +++ b/tests/codegen/inputs/inlining_gh1712_string.d @@ -0,0 +1,6 @@ +import inputs.inlinables; + +void foo() +{ + call_template_foo(1); +}