[cfe-commits] [Patch 4 of 5] Proposed fix for PR12850

[Richard Smith]

On Fri, Aug 24, 2012 at 9:56 PM, Andy Gibbs <andyg1001 at hotmail.co.uk> wrote:

> On Saturday, August 25, 2012 3:51 AM, Richard Smith wrote:
> > Hi Andy,
> >
> > Editorializing somewhat, I think it's unfortunate that people expect
> > constexpr evaluation to be asymptotically more efficient than the runtime
> > evaluation of the same functions, and I'm a little concerned that this
> > caching will lead people to write code which has surprising poor
> > performance in the cases where a constexpr function gets invoked
> > dynamically. But... g++ already implements a similar optimization, so
> > we'll probably need this to handle code written for it.
>
> Oh dear, I'm one of those unfortunate people then :o)
>
> But (as always) I have a use-case: 5 MB of compile-time evaluated lookup
> tables for modelling physical systems.  Yes, these could be evaluated via
> a separate program to generate the lookup tables, but like you say, gcc
> supports this optimisation.  And you're right: hugh performance penalty if
> these functions are ever called at run-time (but there would be anyway,
> hence wanting to do it at compile time)


Well, my point was, the run-time performance penalty could be *much* worse
than the compile-time penalty, since you don't get the caching behavior
there. But I have sympathy for your use case: constexpr does not make it
easy for you to cache and reuse intermediate results yourself.


> > Have you measured the impact of this cache on "well-written" constexpr
> > functions (those not requiring memoization for efficiency)? What's the
> > memory usage and performance impact on, say,
> > test/SemaCXX/constexpr-turing.cpp, with a more complex Turing machine?
> > Can you provide some figures for typical and worst-case slowdowns?
>
> I did do some "informal" testing back when I first worked on this patch,
> but I'll have to do it again so that you have more formal numbers.
>
> From memory, though, yes you have the memory impact (obviously can't
> avoid this), but the speed of compilation is generally not affected, and
> in fact, in some of my testing was actually improved (even on the "well
> written" case) since clang does two evaluations on each constexpr
> function: first to check whether it can be constexpr and then its actual
> use: with this patch some of these cases are only evaluated once since
> the first evaluation is cached.
>
> I have a suggestion though: how about a "cache_constexpr" function
> attribute: then the user can attach this attribute where the feature is
> required and not where it isn't -- then the "well-written" code need
> never change its compile-time characteristics.  (If this were done,
> though, would it be possible somehow to halt evaluation of a constexpr
> function when it had taken more than, say, 10 minutes and produce a
> diagnostic suggesting use of the attribute -- or to consider again the
> algorithm used?! crazy suggestion, yes?)
>
> > There is a nasty corner case where the value of a global changes between
> > successive calls to a constexpr function (this happens if a variable is
> > read after it's zero-initialized but before it's dynamically initialized,
> > from a constexpr function called within its own initializer). Your patch
> > doesn't seem to handle that (though perhaps I just missed it).
>
> No, you're right, I don't think I do handle it.  I was under the obviously
> mistaken impression that constexpr functions were, by their very nature,
> guaranteed to return the same value whenever invoked (assuming the same
> arguments) -- I had understood that this had to be ensured in order to
> determine that it was "safe" to evaluate at compile-time rather than at
> run-time.
>
> Could you give me some example code for your "nasty corner case", and I'll
> fix my code.  Thanks.
>

extern const int n;
constexpr int get() { return n; }
constexpr int n = get() + 1;
constexpr int m = get();
static_assert(m == n, "");
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120824/c25efbd9/attachment.html>