From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8233 invoked by alias); 8 Jul 2019 12:04:56 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 7901 invoked by uid 89); 8 Jul 2019 12:04:56 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-4.6 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.1 spammy=survive, H*f:sk:CAFiYyc, Sure X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 08 Jul 2019 12:04:54 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 4015EAEEC; Mon, 8 Jul 2019 12:04:52 +0000 (UTC) Subject: Re: [PATCH] Deprecate -frepo option. To: Richard Biener , Jakub Jelinek Cc: Jonathan Wakely , Iain Sandoe , "gcc@gcc.gnu.org" , David Edelsohn , Jan Hubicka , GCC Patches References: <884b9feb-3e71-db00-8c72-8e096bf75c1e@suse.cz> <69a11998-93c1-d61d-ba31-ac93bcbb353a@suse.cz> <20190621115838.GX815@tucnak> <79fcb5a4-0eba-d39f-e7ca-389f371cd48c@suse.cz> <20190621141309.GY815@tucnak> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <140a1eb3-9652-9516-0274-19977fa3021d@suse.cz> Date: Mon, 08 Jul 2019 12:14:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-07/txt/msg00571.txt.bz2 On 6/21/19 4:28 PM, Richard Biener wrote: > On Fri, Jun 21, 2019 at 4:13 PM Jakub Jelinek wrote: >> >> On Fri, Jun 21, 2019 at 04:04:00PM +0200, Martin Liška wrote: >>> On 6/21/19 1:58 PM, Jakub Jelinek wrote: >>>> On Fri, Jun 21, 2019 at 01:52:09PM +0200, Martin Liška wrote: >>>>> On 6/21/19 1:47 PM, Jonathan Wakely wrote: >>>>>> On Fri, 21 Jun 2019 at 11:40, Martin Liška wrote: >>>>>>> Yes, I would be fine to deprecate that for GCC 10.1 >>>>>> >>>>>> Would it be appropriate to issue a warning in GCC 10.x if the option is used? >>>>> >>>>> Sure. With the patch attached one will see: >>>>> >>>>> $ gcc -frepo /tmp/main.cc -c >>>>> gcc: warning: switch ‘-frepo’ is no longer supported >>>>> >>>>> I'm sending patch that also removes -frepo tests from test-suite. >>>>> I've been testing the patch. >>>> >>>> IMHO for just deprecation of an option you don't want to remove it from the >>>> testsuite, just match the warning it will generate in those tests, and >>>> I'm not convinced you want to remove it from the documentation (rather than >>>> just saying in the documentation that the option is deprecated and might be >>>> removed in a later GCC version). >>> >>> Agree with you. I'm sending updated version of the patch. >>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> >> I'm also not convinced about the Deprecated flag, seems like that is a flag >> that we use for options that have been already removed. >> So, instead there should be some proper warning in the C++ FE for it, >> or just Warn. > > In principle -frepo is a nice idea - does it live up to its promises? That is, > does it actually work, for example when throwing it on the libstdc++ > testsuite or a larger C++ project? I've just tested tramp3d, and it does not survive linking: g++ tramp3d-v4.o collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking collect: recompiling tramp3d-v4.cpp collect: relinking /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /tmp/ccEeuyj7.ltrans0.ltrans.o: in function `RefCountedBlockPtr, ViewEngine<3, IndexFunction >::PositionsFunctor> > >, false, RefBlockController, ViewEngine<3, IndexFunction >::PositionsFunctor> > > > >::RefCountedBlockPtr(RefCountedBlockPtr, ViewEngine<3, IndexFunction >::PositionsFunctor> > >, false, RefBlockController, ViewEngine<3, IndexFunction >::PositionsFunctor> > > > > const&)': :(.text+0x4181b): undefined reference to `RefCountedPtr, ViewEngine<3, IndexFunction >::PositionsFunctor> > > > >::RefCountedPtr(RefCountedPtr, ViewEngine<3, IndexFunction >::PositionsFunctor> > > > > const&)' /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::_Vector_base, Interval<3> >, std::allocator, Interval<3> > > >::_Vector_impl::~_Vector_impl()': :(.text+0xc1890): undefined reference to `std::allocator, Interval<3> > >::~allocator()' /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::_Vector_base, Interval<3> >, std::allocator, Interval<3> > > >::_Vector_base()': :(.text+0xc18aa): undefined reference to `std::_Vector_base, Interval<3> >, std::allocator, Interval<3> > > >::_Vector_impl::_Vector_impl()' /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::vector, std::allocator > >::_S_use_relocate()': :(.text+0xc496f): undefined reference to `std::vector, std::allocator > >::_S_nothrow_relocate(std::integral_constant)' /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::_Vector_base, Interval<3> >, std::allocator, Interval<3> > > >::~_Vector_base()': :(.text+0xc4cc1): undefined reference to `std::_Vector_base, Interval<3> >, std::allocator, Interval<3> > > >::_M_deallocate(Node, Interval<3> >*, unsigned long)' /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: /tmp/ccEeuyj7.ltrans0.ltrans.o: in function `DataBlockPtr::DataBlockPtr()': :(.text+0xc6f96): undefined reference to `RefCountedBlockPtr >::RefCountedBlockPtr()' [... many other undefined references ...] Same happens also for GCC7. It does 17 iteration (#define MAX_ITERATIONS 17) and apparently 17 is not enough to resolve all symbols. And it's really slow. That said, I would recommend to remove it :) Martin > The option doesn't document > optimization issues but I assume template bodies are not available > for IPA optimizations unless -frepo is combined with LTO where the > template CU[s] then bring them in. > > So I'm not sure - do we really want to remove this feature? > > Richard. > >> Jakub