From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 7F99F3858C50 for ; Mon, 2 May 2022 16:04:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7F99F3858C50 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 613A91F388; Mon, 2 May 2022 16:04:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1651507467; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=6l34lxTF+jvdn4BkwqTDseQhpUZwXUmsjpF/7olZvDM=; b=CrH/04IVK1zMtJ1g+THebuJCUobUvOFVid1rLjQXb8Agtlvoxe1c4Wb6eWPobjnb80LJBr 4ks+MC1jZG7pOdDk2AI8QFPGhEz5xU6gjakaX9HvX0CV8Necr3xEDE+hXIyu7kPiJ5H8Es Dj5d/bwuQebUIDRMoA9cxEONtTYICos= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1651507467; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=6l34lxTF+jvdn4BkwqTDseQhpUZwXUmsjpF/7olZvDM=; b=QOTQ+wN3e0wQnmyqsCAFxDeMCXNdACq0XESMOeCW84FAkdjtP+ONMCamE/afcFGvWjyNtN Lq4+Dju06nPqWDCw== Received: from suse.cz (virgil.suse.cz [10.100.13.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 3B8192C141; Mon, 2 May 2022 16:04:27 +0000 (UTC) From: Martin Jambor To: Artem Klimov Cc: amonakov@ispras.ru, Jan Hubicka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] ipa-visibility: Optimize TLS access [PR99619] In-Reply-To: <20220417185113.25780-1-jakmobius@gmail.com> References: <20220417185113.25780-1-jakmobius@gmail.com> User-Agent: Notmuch/0.35 (https://notmuchmail.org) Emacs/28.1 (x86_64-suse-linux-gnu) Date: Mon, 02 May 2022 18:04:27 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-10.9 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, WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Mon, 02 May 2022 16:04:30 -0000 Hello, On Sun, Apr 17 2022, Artem Klimov via Gcc-patches wrote: > Fix issue PR99619, which asks to optimize TLS access based on > visibility. The fix is implemented as an IPA optimization, which allows > to take optimized visibility status into account (as well as avoid > modifying all language frontends). I'm afraid I'll have to defer to Honza to approve this, but I have found an issue with the patch. > > 2022-04-17 Artem Klimov > > gcc/ChangeLog: > PR middle-end/99619 > * ipa-visibility.cc (function_and_variable_visibility): Add an > explicit TLS model update after visibility optimisation loops. > > gcc/testsuite/ChangeLog: > PR middle-end/99619 > * gcc.dg/tls/vis-attr-gd.c: New test. > * gcc.dg/tls/vis-attr-hidden-gd.c: New test. > * gcc.dg/tls/vis-attr-hidden.c: New test. > * gcc.dg/tls/vis-flag-hidden-gd.c: New test. > * gcc.dg/tls/vis-flag-hidden.c: New test. > * gcc.dg/tls/vis-pragma-hidden-gd.c: New test. > * gcc.dg/tls/vis-pragma-hidden.c: New test. > > Co-Authored-By: Alexander Monakov (Minor nit and I don't care too much, but in GCC we traditionally specify co-authors in the ChangeLog entry beginning by providing more names, one per line. But perhaps we want to adapt more widely used practices.) > Signed-off-by: Artem Klimov > --- [...] > diff --git a/gcc/ipa-visibility.cc b/gcc/ipa-visibility.cc > index e95a0dd252f..ca5b9a95f5e 100644 > --- a/gcc/ipa-visibility.cc > +++ b/gcc/ipa-visibility.cc > @@ -872,6 +872,22 @@ function_and_variable_visibility (bool whole_program) > } > } > } > + FOR_EACH_VARIABLE (vnode) > + { > + tree decl = vnode->decl; > + > + /* Optimize TLS model based on visibility (taking into account > + optimizations done in the preceding loop), unless it was > + specified explicitly. */ > + > + if (DECL_THREAD_LOCAL_P (decl) > + && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl))) > + { > + enum tls_model new_model = decl_default_tls_model (decl); > + gcc_checking_assert (new_model >= decl_tls_model (decl)); > + set_decl_tls_model (decl, new_model); > + } > + } > decl_default_tls_model depends on the global optimize flag, which is almost always problematic in IPA passes. I was able to make your patch ICE using the vis-attr-hidden.c testcase from your patch with: mjambor@virgil:~/gcc/small/tests/tls$ ~/gcc/small/inst/bin/gcc -O2 -fPIC -flto -c vis-attr-hidden.c mjambor@virgil:~/gcc/small/tests/tls$ ~/gcc/small/inst/bin/gcc -fPIC -O0 -shared -flto vis-attr-hidden.o during IPA pass: whole-program lto1: internal compiler error: in function_and_variable_visibility, at ipa-visibility.cc:888 0x25f48c0 function_and_variable_visibility /home/mjambor/gcc/small/src/gcc/ipa-visibility.cc:888 0x25f4b33 whole_program_function_and_variable_visibility /home/mjambor/gcc/small/src/gcc/ipa-visibility.cc:938 0x25f4bda execute /home/mjambor/gcc/small/src/gcc/ipa-visibility.cc:986 Please submit a full bug report, with preprocessed source (by using -freport-bug). Please include the complete backtrace with any bug report. See for instructions. lto-wrapper: fatal error: /home/mjambor/gcc/small/inst/bin/gcc returned 1 exit status compilation terminated. /usr/bin/ld: error: lto-wrapper failed collect2: error: ld returned 1 exit status Note the use of LTO, mismatching -O flags and the -shared flag in the link step. A simple but somewhat lame way to avoid the ICE would be to run your loop over variables only from pass_ipa_function_and_variable_visibility and not from pass_ipa_whole_program_visibility. I am afraid a real solution would involve copying relevant entries from global_options to the symtab node representing the variable when it is created/finalized, properly streaming them for LTO, and modifying decl_default_tls_model to rely on those rather than global_options itself. But maybe Honza has some other clever idea. Also, please be careful not to unnecessarily commit trailing blank spaces, the empty lines in your patch are not really empty. Martin