This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Deprecate -frepo option.
- From: Martin Liška <mliska at suse dot cz>
- To: Richard Biener <richard dot guenther at gmail dot com>, Jakub Jelinek <jakub at redhat dot com>
- Cc: Jonathan Wakely <jwakely dot gcc at gmail dot com>, Iain Sandoe <idsandoe at googlemail dot com>, "gcc at gcc dot gnu dot org" <gcc at gcc dot gnu dot org>, David Edelsohn <dje dot gcc at gmail dot com>, Jan Hubicka <hubicka at ucw dot cz>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 8 Jul 2019 14:04:51 +0200
- Subject: Re: [PATCH] Deprecate -frepo option.
- References: <ac43bd46-9251-c360-b92c-52831886fc4a@suse.cz> <BC1522A8-E989-448F-BB2D-7552F1111F1D@gmail.com> <884b9feb-3e71-db00-8c72-8e096bf75c1e@suse.cz> <CAH6eHdS-oVx-pSwsFwsnsH02biaLPyrhzv-8V23JtpfXa1=WBA@mail.gmail.com> <B2D95072-A88E-4BAA-9F33-E46685EDFE9A@googlemail.com> <b8d94fbd-dc73-ad66-17ca-ce34238581e3@suse.cz> <CAH6eHdTPwE+6xv7n6HMsCmH-Ei=zTLN+vmAB-51=CzytmjEJFQ@mail.gmail.com> <69a11998-93c1-d61d-ba31-ac93bcbb353a@suse.cz> <20190621115838.GX815@tucnak> <79fcb5a4-0eba-d39f-e7ca-389f371cd48c@suse.cz> <20190621141309.GY815@tucnak> <CAFiYyc3mw2hitAyp-Xfucv5sAM6TbmTyHyicGTKWGSi5znPrgA@mail.gmail.com>
On 6/21/19 4:28 PM, Richard Biener wrote:
> On Fri, Jun 21, 2019 at 4:13 PM Jakub Jelinek <jakub@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.
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