On Sat, Jun 29, 2019 at 10:31 AM Bruno Haible <br...@clisp.org> wrote: > Pip Cet wrote: > > I would like to state that the current assume does > > behave very badly when combined with -fno-inline-small-functions > > -fno-inline. > > Since we can't address this limitation through an acceptable change > to the 'assume' macro, we need to address it through documentation.
I agree. There's a limitation bad enough for me to consider it a bug in current GCC, and even if that's lifted tomorrow, it wouldn't be backported so it would be quite a while until I'd ask you to reconsider the matter. > > marking it > > as __attribute__((always_inline)) is problematic because it might > > directly contradict what the programmer was trying to achieve by > > passing -fno-inline. > > __attribute__((always_inline)) exists precisely to make a distinction > between functions where inlining is usually desirable vs. functions > where inlining is essential. Indeed. In this case, it's usually desirable but not essential for the correctness of the program, IMHO. I understand you disagree. > We don't need to warn against the uses > of __attribute__((always_inline)) -- confusing behaviour in the debugger > etc. -- becauses these drawbacks are already well-known. I disagree completely. There's no warning in the GCC documentation for the attribute. > As a user of the 'assume' macro, I want a definitive statement about what > I need to provide so that the macro works in the sense of improved > performance. > A statement about "often" is too vague. I agree. > How about this proposed patch? > > diff --git a/lib/verify.h b/lib/verify.h > index f8e4eff..ed1ba19 100644 > --- a/lib/verify.h > +++ b/lib/verify.h > @@ -261,7 +261,10 @@ template <int w> > > /* Assume that R always holds. This lets the compiler optimize > accordingly. R should not have side-effects; it may or may not be > - evaluated. Behavior is undefined if R is false. */ > + evaluated. The behavior is undefined if R is false. > + If you want the use of this macro to improve, not deteriorate, > + performance, R should not contain function calls except to functions > + that are declared 'inline __attribute__((__always_inline__))'. */ My suggestion would be "Assume that R always holds. This lets the compiler optimize accordingly. Behavior is undefined if R is false, fails to evaluate, or has side effects. Performance will suffer if R contains function calls that are not inlined at compile time." That would describe the API as I understand you and Paul think it always has been; I think it describes a drastically stricter API than what the old comment did. As a reminder, my starting point was that I wanted to make the API more lenient. So, obviously, I disagree with the API change but it is more important that there's consensus on what the API actually is than it is to have a good API.
