On Mon, Jul 8, 2019 at 2:04 PM Martin Liška <mli...@suse.cz> wrote: > > On 6/21/19 4:28 PM, Richard Biener wrote: > > On Fri, Jun 21, 2019 at 4:13 PM Jakub Jelinek <ja...@redhat.com> 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<FieldEngineBaseData<3, Vector<3, double, Full>, > ViewEngine<3, IndexFunction<GenericURM<MeshTraits<3, double, > UniformRectilinearTag, CartesianTag, 3> >::PositionsFunctor> > >, false, > RefBlockController<FieldEngineBaseData<3, Vector<3, double, Full>, > ViewEngine<3, IndexFunction<GenericURM<MeshTraits<3, double, > UniformRectilinearTag, CartesianTag, 3> >::PositionsFunctor> > > > > >::RefCountedBlockPtr(RefCountedBlockPtr<FieldEngineBaseData<3, Vector<3, > double, Full>, ViewEngine<3, IndexFunction<GenericURM<MeshTraits<3, double, > UniformRectilinearTag, CartesianTag, 3> >::PositionsFunctor> > >, false, > RefBlockController<FieldEngineBaseData<3, Vector<3, double, Full>, > ViewEngine<3, IndexFunction<GenericURM<MeshTraits<3, double, > UniformRectilinearTag, CartesianTag, 3> >::PositionsFunctor> > > > > const&)': > <artificial>:(.text+0x4181b): undefined reference to > `RefCountedPtr<RefBlockController<FieldEngineBaseData<3, Vector<3, double, > Full>, ViewEngine<3, IndexFunction<GenericURM<MeshTraits<3, double, > UniformRectilinearTag, CartesianTag, 3> >::PositionsFunctor> > > > > >::RefCountedPtr(RefCountedPtr<RefBlockController<FieldEngineBaseData<3, > Vector<3, double, Full>, ViewEngine<3, IndexFunction<GenericURM<MeshTraits<3, > double, UniformRectilinearTag, CartesianTag, 3> >::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<Node<Range<3>, > Interval<3> >, std::allocator<Node<Range<3>, Interval<3> > > > >::_Vector_impl::~_Vector_impl()': > <artificial>:(.text+0xc1890): undefined reference to > `std::allocator<Node<Range<3>, 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<Node<Range<3>, > Interval<3> >, std::allocator<Node<Range<3>, Interval<3> > > > >::_Vector_base()': > <artificial>:(.text+0xc18aa): undefined reference to > `std::_Vector_base<Node<Range<3>, Interval<3> >, > std::allocator<Node<Range<3>, 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<INode<3>, > std::allocator<INode<3> > >::_S_use_relocate()': > <artificial>:(.text+0xc496f): undefined reference to `std::vector<INode<3>, > std::allocator<INode<3> > >::_S_nothrow_relocate(std::integral_constant<bool, > true>)' > /usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: > /tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::_Vector_base<Node<Range<3>, > Interval<3> >, std::allocator<Node<Range<3>, Interval<3> > > > >::~_Vector_base()': > <artificial>:(.text+0xc4cc1): undefined reference to > `std::_Vector_base<Node<Range<3>, Interval<3> >, > std::allocator<Node<Range<3>, Interval<3> > > >::_M_deallocate(Node<Range<3>, > 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<double, > false>::DataBlockPtr()': > <artificial>:(.text+0xc6f96): undefined reference to > `RefCountedBlockPtr<double, false, DataBlockController<double> > >::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.
Ouch. > That said, I would recommend to remove it :) In the end it's up to the C++ FE maintainers but the above clearly doesn't look promising (not sure if it keeps re-compiling _all_ repo-triggered templates or just incrementally adds them to new object files). Also the docs suggest that -frepo is the only way to get template instantiation commoning if the target object format doesn't support sth like linkonce or comdats. Not sure if we need to care though. I don't remember any bugs about -frepo in many last years but that could be because it's perfect (well, it is not as seen above, but...). I'm not opposed to removing -frepo from GCC 10 but then I would start noting it is obsolete on the GCC 9 branch at least. Jason/Nathan? Thanks, Richard. > 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 >
