From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout-p-102.mailbox.org (mout-p-102.mailbox.org [IPv6:2001:67c:2050:0:465::102]) by sourceware.org (Postfix) with ESMTPS id DC989385734F for ; Tue, 9 Aug 2022 12:43:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DC989385734F Received: from smtp102.mailbox.org (smtp102.mailbox.org [IPv6:2001:67c:2050:b231:465::102]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-102.mailbox.org (Postfix) with ESMTPS id 4M2CQz4nmQz9sVm; Tue, 9 Aug 2022 14:43:07 +0200 (CEST) From: Iain Buclaw To: gcc-patches@gcc.gnu.org Subject: [committed] d: Fix undefined reference to pragma(inline) symbol (PR106563) Date: Tue, 9 Aug 2022 14:43:05 +0200 Message-Id: <20220809124305.2715498-1-ibuclaw@gdcproject.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 4M2CQz4nmQz9sVm X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Aug 2022 12:43:17 -0000 Hi, This patch changes the emission strategy for inline functions so that they are given codegen in every referencing module, not just the module that they are defined in. Functions that are declared `pragma(inline)' should be treated as if they are defined in every translation unit they are referenced from, regardless of visibility protection. Ensure they always get DECL_ONE_ONLY linkage, and start emitting them into other modules that import them. Bootstrapped and regression tested on x86_64-linux-gnu/-m32/-mx32, committed to mainline and backported to releases/gcc-12. Regards, Iain. --- PR d/106563 gcc/d/ChangeLog: * decl.cc (DeclVisitor::visit (FuncDeclaration *)): Set semanticRun before generating its symbol. (function_defined_in_root_p): New function. (function_needs_inline_definition_p): New function. (maybe_build_decl_tree): New function. (get_symbol_decl): Call maybe_build_decl_tree before returning symbol. (start_function): Use function_defined_in_root_p instead of inline test for locally defined symbols. (set_linkage_for_decl): Check for inline functions before private or protected symbols. gcc/testsuite/ChangeLog: * gdc.dg/torture/torture.exp (srcdir): New proc. * gdc.dg/torture/imports/pr106563math.d: New test. * gdc.dg/torture/imports/pr106563regex.d: New test. * gdc.dg/torture/imports/pr106563uni.d: New test. * gdc.dg/torture/pr106563.d: New test. --- gcc/d/decl.cc | 121 +++++++++++++++--- .../gdc.dg/torture/imports/pr106563math.d | 12 ++ .../gdc.dg/torture/imports/pr106563regex.d | 7 + .../gdc.dg/torture/imports/pr106563uni.d | 15 +++ gcc/testsuite/gdc.dg/torture/pr106563.d | 16 +++ gcc/testsuite/gdc.dg/torture/torture.exp | 9 ++ 6 files changed, 161 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/gdc.dg/torture/imports/pr106563math.d create mode 100644 gcc/testsuite/gdc.dg/torture/imports/pr106563regex.d create mode 100644 gcc/testsuite/gdc.dg/torture/imports/pr106563uni.d create mode 100644 gcc/testsuite/gdc.dg/torture/pr106563.d diff --git a/gcc/d/decl.cc b/gcc/d/decl.cc index 43c3d87cdd1..9119323175e 100644 --- a/gcc/d/decl.cc +++ b/gcc/d/decl.cc @@ -823,6 +823,10 @@ public: if (global.errors) return; + /* Start generating code for this function. */ + gcc_assert (d->semanticRun == PASS::semantic3done); + d->semanticRun = PASS::obj; + /* Duplicated FuncDeclarations map to the same symbol. Check if this is the one declaration which will be emitted. */ tree fndecl = get_symbol_decl (d); @@ -839,10 +843,6 @@ public: if (global.params.verbose) message ("function %s", d->toPrettyChars ()); - /* Start generating code for this function. */ - gcc_assert (d->semanticRun == PASS::semantic3done); - d->semanticRun = PASS::obj; - tree old_context = start_function (d); tree parm_decl = NULL_TREE; @@ -1015,13 +1015,103 @@ build_decl_tree (Dsymbol *d) input_location = saved_location; } +/* Returns true if function FD is defined or instantiated in a root module. */ + +static bool +function_defined_in_root_p (FuncDeclaration *fd) +{ + Module *md = fd->getModule (); + if (md && md->isRoot ()) + return true; + + TemplateInstance *ti = fd->isInstantiated (); + if (ti && ti->minst && ti->minst->isRoot ()) + return true; + + return false; +} + +/* Returns true if function FD always needs to be implicitly defined, such as + it was declared `pragma(inline)'. */ + +static bool +function_needs_inline_definition_p (FuncDeclaration *fd) +{ + /* Function has already been defined. */ + if (!DECL_EXTERNAL (fd->csym)) + return false; + + /* Non-inlineable functions are always external. */ + if (DECL_UNINLINABLE (fd->csym)) + return false; + + /* No function body available for inlining. */ + if (!fd->fbody) + return false; + + /* Ignore functions that aren't decorated with `pragma(inline)'. */ + if (fd->inlining != PINLINE::always) + return false; + + /* These functions are tied to the module they are defined in. */ + if (fd->isFuncLiteralDeclaration () + || fd->isUnitTestDeclaration () + || fd->isFuncAliasDeclaration () + || fd->isInvariantDeclaration ()) + return false; + + /* Check whether function will be regularly defined later in the current + translation unit. */ + if (function_defined_in_root_p (fd)) + return false; + + /* Weak functions cannot be inlined. */ + if (lookup_attribute ("weak", DECL_ATTRIBUTES (fd->csym))) + return false; + + /* Naked functions cannot be inlined. */ + if (lookup_attribute ("naked", DECL_ATTRIBUTES (fd->csym))) + return false; + + return true; +} + +/* If the variable or function declaration in DECL needs to be defined, call + build_decl_tree on it now before returning its back-end symbol. */ + +static tree +maybe_build_decl_tree (Declaration *decl) +{ + gcc_assert (decl->csym != NULL_TREE); + + /* Still running semantic analysis on declaration, or it has already had its + code generated. */ + if (doing_semantic_analysis_p || decl->semanticRun >= PASS::obj) + return decl->csym; + + if (error_operand_p (decl->csym)) + return decl->csym; + + if (FuncDeclaration *fd = decl->isFuncDeclaration ()) + { + /* Externally defined inline functions need to be emitted. */ + if (function_needs_inline_definition_p (fd)) + { + DECL_EXTERNAL (fd->csym) = 0; + build_decl_tree (fd); + } + } + + return decl->csym; +} + /* Return the decl for the symbol, create it if it doesn't already exist. */ tree get_symbol_decl (Declaration *decl) { if (decl->csym) - return decl->csym; + return maybe_build_decl_tree (decl); /* Deal with placeholder symbols immediately: SymbolDeclaration is used as a shell around an initializer symbol. */ @@ -1397,7 +1487,7 @@ get_symbol_decl (Declaration *decl) TREE_USED (decl->csym) = 1; d_keep (decl->csym); - return decl->csym; + return maybe_build_decl_tree (decl); } /* Returns a declaration for a VAR_DECL. Used to create compiler-generated @@ -1892,15 +1982,8 @@ start_function (FuncDeclaration *fd) /* Function has been defined, check now whether we intend to send it to object file, or it really is extern. Such as inlinable functions from modules not in this compilation, or thunk aliases. */ - TemplateInstance *ti = fd->isInstantiated (); - if (ti && ti->needsCodegen ()) + if (function_defined_in_root_p (fd)) DECL_EXTERNAL (fndecl) = 0; - else - { - Module *md = fd->getModule (); - if (md && md->isRoot ()) - DECL_EXTERNAL (fndecl) = 0; - } DECL_INITIAL (fndecl) = error_mark_node; @@ -2421,16 +2504,16 @@ set_linkage_for_decl (tree decl) if (!TREE_PUBLIC (decl)) return; - /* Don't need to give private or protected symbols a special linkage. */ - if ((TREE_PRIVATE (decl) || TREE_PROTECTED (decl)) - && !DECL_INSTANTIATED (decl)) - return; - /* Functions declared as `pragma(inline, true)' can appear in multiple translation units. */ if (TREE_CODE (decl) == FUNCTION_DECL && DECL_DECLARED_INLINE_P (decl)) return d_comdat_linkage (decl); + /* Don't need to give private or protected symbols a special linkage. */ + if ((TREE_PRIVATE (decl) || TREE_PROTECTED (decl)) + && !DECL_INSTANTIATED (decl)) + return; + /* If all instantiations must go in COMDAT, give them that linkage. This also applies to other extern declarations, so that it is possible for them to override template declarations. */ diff --git a/gcc/testsuite/gdc.dg/torture/imports/pr106563math.d b/gcc/testsuite/gdc.dg/torture/imports/pr106563math.d new file mode 100644 index 00000000000..b9351ea74e2 --- /dev/null +++ b/gcc/testsuite/gdc.dg/torture/imports/pr106563math.d @@ -0,0 +1,12 @@ +module imports.pr106563math; + +T nextPow2(T)(const T val) +{ + return powIntegralImpl(val); +} + +pragma(inline, true) +T powIntegralImpl(T)(T) +{ + return 1; +} diff --git a/gcc/testsuite/gdc.dg/torture/imports/pr106563regex.d b/gcc/testsuite/gdc.dg/torture/imports/pr106563regex.d new file mode 100644 index 00000000000..a2cd90c3d8b --- /dev/null +++ b/gcc/testsuite/gdc.dg/torture/imports/pr106563regex.d @@ -0,0 +1,7 @@ +module imports.pr106563regex; +import imports.pr106563uni; + +struct CharMatcher +{ + typeof(MultiArray!().length) trie; +} diff --git a/gcc/testsuite/gdc.dg/torture/imports/pr106563uni.d b/gcc/testsuite/gdc.dg/torture/imports/pr106563uni.d new file mode 100644 index 00000000000..16e3bc85cc0 --- /dev/null +++ b/gcc/testsuite/gdc.dg/torture/imports/pr106563uni.d @@ -0,0 +1,15 @@ +module imports.pr106563uni; + +struct MultiArray() +{ + @property length() + { + return spaceFor!0(); + } +} + +size_t spaceFor(size_t bits)() +{ + import imports.pr106563math; + return nextPow2(bits); +} diff --git a/gcc/testsuite/gdc.dg/torture/pr106563.d b/gcc/testsuite/gdc.dg/torture/pr106563.d new file mode 100644 index 00000000000..7e15442c0bb --- /dev/null +++ b/gcc/testsuite/gdc.dg/torture/pr106563.d @@ -0,0 +1,16 @@ +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106563 +// { dg-do link } +// { dg-additional-files "imports/pr106563math.d imports/pr106563regex.d imports/pr106563uni.d" } +// { dg-additional-options "-I[srcdir] -fno-druntime" } +import imports.pr106563math; +import imports.pr106563regex; + +auto requireSize()(size_t size) +{ + return nextPow2(size); +} + +extern(C) int main() +{ + return cast(int)requireSize(0); +} diff --git a/gcc/testsuite/gdc.dg/torture/torture.exp b/gcc/testsuite/gdc.dg/torture/torture.exp index f7d00b1b052..d9c6a79cfe2 100644 --- a/gcc/testsuite/gdc.dg/torture/torture.exp +++ b/gcc/testsuite/gdc.dg/torture/torture.exp @@ -19,6 +19,15 @@ # Load support procs. load_lib gdc-dg.exp +# Helper function allows adding tests that use imports/*, but don't compile +# the sources in with dg-additional-sources. +global testdir +set testdir $srcdir/$subdir +proc srcdir {} { + global testdir + return $testdir +} + # The default option list can be overridden by # TORTURE_OPTIONS="{ { list1 } ... { listN } }" -- 2.34.1