This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)
- From: Jonathan Wakely <jwakely at redhat dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jonathan Wakely <jwakely dot gcc at gmail dot com>, libstdc++ at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org, Marc Glisse <marc dot glisse at inria dot fr>
- Date: Thu, 10 Jan 2019 10:52:25 +0000
- Subject: Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)
- References: <20190110090219.GT30353@tucnak>
On 10/01/19 10:02 +0100, Jakub Jelinek wrote:
Hi!
In Marc's testcase, we generate terrible code for std::string assignment,
because the __builtin_constant_p is kept in the IL for way too long and the
optimizers (jump threading?) create way too many copies of the
memcpy/memmove calls that it is then hard to bring it back in sanitity.
On the testcase in the PR, GCC 7 emits on x86_64 with -O2 99 bytes long
function, GCC 9 unpatched 259 bytes long, with this patch it emits
139 bytes long, better but still not as good as before. I guess we'll need
to improve GIMPLE optimizers too, but having twice as small IL for these
heavily used operators where e.g. _M_disjunct uses two of them and we wind
up with twice as many branches because of that is IMHO very useful.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
1) I'm not really sure about proper formatting in libstdc++, I thought you
don't use space before ( in function calls, but then why is there a space
in __builtin_constant_p?
I guess I probably copy&pasted it from something you gave me :-)
The space shouldn't be there.
2) not really sure about that #if __cplusplus >= 201402L either, I think we
don't really want to use __builtin_is_constant_evaluated at least in
C++98 code, but even in C++11, if the operator isn't constexpr, is there
any point trying to help it do the right thing in constexpr contexts?
I think there's no point, so only doing it for C++14 and later looks
OK to me.
OK for trunk, thanks.