bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: bug#36370: 27.0.50; XFIXNAT called on negative numbers


From: Bruno Haible
Subject: Re: bug#36370: 27.0.50; XFIXNAT called on negative numbers
Date: Sat, 29 Jun 2019 01:30:06 +0200
User-agent: KMail/5.1.3 (Linux/4.4.0-151-generic; KDE/5.18.0; x86_64; ; )

Pip Cet wrote:
> have started believing the "an inline function is as fast as a macro"
> mantra*, assuming you include inline functions with "function calls".

Ah, that's where the entire topic with the function calls inside assume()
comes from! I agree it's an important case (more important than the
functions defined in other compilation units). So let's test this:

==================================== foo.c ====================================
#include <stdio.h>

#define assume(R) ((R) ? (void) 0 : __builtin_unreachable ())
//#define assume(R) (!__builtin_constant_p (!(R) == !(R)) || (R) ? (void) 0 : 
__builtin_unreachable ())

#if USE_MACROS
# define complicated(i) (((i) & 7) == 3)
# define nonnegative(i) ((i) >= 0)
#else
static inline int complicated (int i) { return (i & 7) == 3; }
static inline int nonnegative (int i) { return i >= 0; }
#endif

#if COMPLEX_CONDITION
# define CONDITION complicated (i) && nonnegative (i)
#else
# define CONDITION nonnegative (i)
#endif

int f_generic (int i)
{
  printf("%d\n", i & 0x80000000);
  return 0;
}

int f_condition (int i)
{
  if (CONDITION)
    printf("%d\n", i & 0x80000000);
  return 0;
}

int f_assume (int i)
{
  assume (CONDITION);
  printf("%d\n", i & 0x80000000);
  return 0;
}
===============================================================================
$ gcc -O2 -m32 -S foo.c && fgrep -v .cfi foo.s

Results:
// old 'assume', !COMPLEX_CONDITION, USE_MACROS  -> f_assume optimized
// old 'assume', COMPLEX_CONDITION,  USE_MACROS  -> f_assume optimized
// old 'assume', !COMPLEX_CONDITION, !USE_MACROS -> f_assume optimized
// old 'assume', COMPLEX_CONDITION,  !USE_MACROS -> f_assume optimized
// new 'assume', !COMPLEX_CONDITION, USE_MACROS  -> f_assume optimized
// new 'assume', COMPLEX_CONDITION,  USE_MACROS  -> f_assume optimized
// new 'assume', !COMPLEX_CONDITION, !USE_MACROS -> f_assume not optimized
// new 'assume', COMPLEX_CONDITION,  !USE_MACROS -> f_assume not optimized

So, the main effect of the proposed new 'assume' is that it de-optimizes
the case where the CONDITION is defined using inline functions!

The other case - that the CONDITION calls functions defined in other
compilation units - is a fringe case. And the topic regarding the
COMPLEX_CONDITION versus simple condition is also less important.

Based on these results, I formally object against the proposed patch.

> > (2) that the generated code will never include these function calls,
> >     because the generated code with the 'assume' invocation should be
> >     optimized at least as well as the generated code without the
> >     'assume' invocation.
> 
> I think it should be the rarest of exceptions for an assume() to
> result in slower code, yes. I believe that includes the case where
> functions marked inline aren't inlined, because of compiler options,
> for example.

Then, I think we should change the documentation of 'assume' to say
that when it invokes functions, these functions should be marked
'__attribute__ ((__always_inline__))', otherwise performance will
be worse than without the 'assume', not better.

> (1) implement the documented API, and don't change it
> (2) when optimizing for speed, do not produce slower code with
> eassume() than we would without it. Even when the programmer wrongly
> guessed that a function would be inlined.
> (3) when optimizing for size, do not produce larger code with
> eassume() than we would without it. Even when inline functions are not
> inlined.
> (4) be at least as fast as gnulib assume()

You evidently have slightly different quality criteria than I do. :)

> > I believe the only way to attain the goals and the quality criteria
> > is, as you suggested, to ask the GCC people to add a __builtin_assume
> > built-in.
> 
> I think there's a significant probability that the GCC people would
> agree to add such a built-in, but insist on its having "may or may not
> evaluate its argument" semantics.

We can tell them that it would be important for us that is does not
evaluate its argument. Like sizeof (EXPRESSION) does not evaluate EXPRESSION.

> Sorry if I'm being a bit dense here.

No problem. I'm also often being dense.

Bruno




reply via email to

[Prev in Thread] Current Thread [Next in Thread]