[wwwdocs] Typo in description of __builtin_expect_with_probability

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[wwwdocs] Typo in description of __builtin_expect_with_probability

Alexander Oblovatniy
Hello,

I'd like to report a typo in description of
«__builtin_expect_with_probability»:
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins

The description starts with "The built-in has same semantics as
*__builtin_expect_with_probability*", but it seems like *__builtin_expect*
should be there.

Thanks!
Reply | Threaded
Open this post in threaded view
|

Re: [wwwdocs] Typo in description of __builtin_expect_with_probability

Martin Liška-2
On 10/30/18 3:56 AM, Alexander Oblovatniy wrote:
> Hello,
>
> I'd like to report a typo in description of
> «__builtin_expect_with_probability»:
> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins
>
> The description starts with "The built-in has same semantics as
> *__builtin_expect_with_probability*", but it seems like *__builtin_expect*
> should be there.

Thanks for reporting the issue. It's fixed as r265615.

Martin

>
> Thanks!
>

Reply | Threaded
Open this post in threaded view
|

Re: [wwwdocs] Typo in description of __builtin_expect_with_probability

Rasmus Villemoes-2
On 2018-10-30 09:29, Martin Liška wrote:

> On 10/30/18 3:56 AM, Alexander Oblovatniy wrote:
>> Hello,
>>
>> I'd like to report a typo in description of
>> «__builtin_expect_with_probability»:
>> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins
>>
>> The description starts with "The built-in has same semantics as
>> *__builtin_expect_with_probability*", but it seems like *__builtin_expect*
>> should be there.
>
> Thanks for reporting the issue. It's fixed as r265615.

Other issues around the same place: "expected probability (in percent)"
seems to contradict "valid values are in inclusive range 0.0f and 1.0f".
The prototype is listed as

(long exp, long c, long probability)

contradicting the prose "Last argument probability is of float type".
So, should probability be an integer in [0, 100], or a float in [0, 1]?

Rasmus
Reply | Threaded
Open this post in threaded view
|

Re: [wwwdocs] Typo in description of __builtin_expect_with_probability

Jonathan Wakely-4
On Tue, 30 Oct 2018 at 09:18, Rasmus Villemoes <[hidden email]> wrote:

>
> On 2018-10-30 09:29, Martin Liška wrote:
> > On 10/30/18 3:56 AM, Alexander Oblovatniy wrote:
> >> Hello,
> >>
> >> I'd like to report a typo in description of
> >> «__builtin_expect_with_probability»:
> >> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins
> >>
> >> The description starts with "The built-in has same semantics as
> >> *__builtin_expect_with_probability*", but it seems like *__builtin_expect*
> >> should be there.
> >
> > Thanks for reporting the issue. It's fixed as r265615.
>
> Other issues around the same place: "expected probability (in percent)"
> seems to contradict "valid values are in inclusive range 0.0f and 1.0f".
> The prototype is listed as
>
> (long exp, long c, long probability)

The testcases added alongside the new built-in use float arguments,
but the actual definition of the built-in seems to use a long double.

What does "user can provide expected probability (in percent) for
value of @var{exp}" mean? What is the probability of the value of exp?
Don't you mean the probability that exp==c?

Also the grammar in the text needs several fixes.




>
> contradicting the prose "Last argument probability is of float type".
> So, should probability be an integer in [0, 100], or a float in [0, 1]?
>
> Rasmus
Reply | Threaded
Open this post in threaded view
|

Re: [wwwdocs] Typo in description of __builtin_expect_with_probability

Jonathan Wakely-4
On Tue, 30 Oct 2018 at 09:54, Jonathan Wakely <[hidden email]> wrote:

>
> On Tue, 30 Oct 2018 at 09:18, Rasmus Villemoes <[hidden email]> wrote:
> >
> > On 2018-10-30 09:29, Martin Liška wrote:
> > > On 10/30/18 3:56 AM, Alexander Oblovatniy wrote:
> > >> Hello,
> > >>
> > >> I'd like to report a typo in description of
> > >> «__builtin_expect_with_probability»:
> > >> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins
> > >>
> > >> The description starts with "The built-in has same semantics as
> > >> *__builtin_expect_with_probability*", but it seems like *__builtin_expect*
> > >> should be there.
> > >
> > > Thanks for reporting the issue. It's fixed as r265615.
> >
> > Other issues around the same place: "expected probability (in percent)"
> > seems to contradict "valid values are in inclusive range 0.0f and 1.0f".
> > The prototype is listed as
> >
> > (long exp, long c, long probability)
>
> The testcases added alongside the new built-in use float arguments,
> but the actual definition of the built-in seems to use a long double.
>
> What does "user can provide expected probability (in percent) for
> value of @var{exp}" mean? What is the probability of the value of exp?
> Don't you mean the probability that exp==c?
>
> Also the grammar in the text needs several fixes.

Maybe something like:

--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -12025,12 +12025,12 @@ when testing pointer or floating-point values.
 @end deftypefn

 @deftypefn {Built-in Function} long __builtin_expect_with_probability
-(long @var{exp}, long @var{c}, long @var{probability})
+(long @var{exp}, long @var{c}, long double @var{probability})

-The built-in has same semantics as @code{__builtin_expect},
-but user can provide expected probability (in percent) for value of @var{exp}.
-Last argument @var{probability} is of float type and valid values
-are in inclusive range 0.0f and 1.0f.
+This function has the same semantics as @code{__builtin_expect},
+but the caller provides the expected probability that @var{exp} == @var{c}.
+The last argument, @var{probability}, is a floating point value in the
+range 0.0 to 1.0, inclusive.
 @end deftypefn

 @deftypefn {Built-in Function} void __builtin_trap (void)
Reply | Threaded
Open this post in threaded view
|

Re: [wwwdocs] Typo in description of __builtin_expect_with_probability

Rasmus Villemoes-2
In reply to this post by Jonathan Wakely-4
On 2018-10-30 10:54, Jonathan Wakely wrote:

> On Tue, 30 Oct 2018 at 09:18, Rasmus Villemoes <[hidden email]> wrote:
>>
>> On 2018-10-30 09:29, Martin Liška wrote:
>>> On 10/30/18 3:56 AM, Alexander Oblovatniy wrote:
>>>> Hello,
>>>>
>>>> I'd like to report a typo in description of
>>>> «__builtin_expect_with_probability»:
>>>> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins
>>>>
>>>> The description starts with "The built-in has same semantics as
>>>> *__builtin_expect_with_probability*", but it seems like *__builtin_expect*
>>>> should be there.
>>>
>>> Thanks for reporting the issue. It's fixed as r265615.
>>
>> Other issues around the same place: "expected probability (in percent)"
>> seems to contradict "valid values are in inclusive range 0.0f and 1.0f".
>> The prototype is listed as
>>
>> (long exp, long c, long probability)
>
> The testcases added alongside the new built-in use float arguments,
> but the actual definition of the built-in seems to use a long double.

In

DEF_FUNCTION_TYPE_3 (BT_FN_LONG_LONG_LONG_DOUBLE,
               BT_LONG, BT_LONG, BT_LONG, BT_DOUBLE)


doesn't the first LONG refer to the return type, so that the third
argument just has type double? Other than that, your suggested update
makes sense (provided it actually matches the implementation, which I
can't verify).
Reply | Threaded
Open this post in threaded view
|

Re: [wwwdocs] Typo in description of __builtin_expect_with_probability

Jakub Jelinek
On Tue, Oct 30, 2018 at 11:11:00AM +0100, Rasmus Villemoes wrote:
> In
>
> DEF_FUNCTION_TYPE_3 (BT_FN_LONG_LONG_LONG_DOUBLE,
>                BT_LONG, BT_LONG, BT_LONG, BT_DOUBLE)
>
>
> doesn't the first LONG refer to the return type, so that the third
> argument just has type double? Other than that, your suggested update

Yes.  A long double argument would be BT_LONGDOUBLE or written as _LONGDOUBLE
in the name of the fn type.

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: [wwwdocs] Typo in description of __builtin_expect_with_probability

Martin Liška-2
In reply to this post by Jonathan Wakely-4
On 10/30/18 10:58 AM, Jonathan Wakely wrote:

> On Tue, 30 Oct 2018 at 09:54, Jonathan Wakely <[hidden email]> wrote:
>>
>> On Tue, 30 Oct 2018 at 09:18, Rasmus Villemoes <[hidden email]> wrote:
>>>
>>> On 2018-10-30 09:29, Martin Liška wrote:
>>>> On 10/30/18 3:56 AM, Alexander Oblovatniy wrote:
>>>>> Hello,
>>>>>
>>>>> I'd like to report a typo in description of
>>>>> «__builtin_expect_with_probability»:
>>>>> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins
>>>>>
>>>>> The description starts with "The built-in has same semantics as
>>>>> *__builtin_expect_with_probability*", but it seems like *__builtin_expect*
>>>>> should be there.
>>>>
>>>> Thanks for reporting the issue. It's fixed as r265615.
>>>
>>> Other issues around the same place: "expected probability (in percent)"
>>> seems to contradict "valid values are in inclusive range 0.0f and 1.0f".
>>> The prototype is listed as
>>>
>>> (long exp, long c, long probability)
>>
>> The testcases added alongside the new built-in use float arguments,
>> but the actual definition of the built-in seems to use a long double.
>>
>> What does "user can provide expected probability (in percent) for
>> value of @var{exp}" mean? What is the probability of the value of exp?
>> Don't you mean the probability that exp==c?
>>
>> Also the grammar in the text needs several fixes.
>
> Maybe something like:
>
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -12025,12 +12025,12 @@ when testing pointer or floating-point values.
>  @end deftypefn
>
>  @deftypefn {Built-in Function} long __builtin_expect_with_probability
> -(long @var{exp}, long @var{c}, long @var{probability})
> +(long @var{exp}, long @var{c}, long double @var{probability})

Hi.

Thanks for rewording!

I guess we should use:
s/long double/double

>
> -The built-in has same semantics as @code{__builtin_expect},
> -but user can provide expected probability (in percent) for value of @var{exp}.
> -Last argument @var{probability} is of float type and valid values
> -are in inclusive range 0.0f and 1.0f.
> +This function has the same semantics as @code{__builtin_expect},
> +but the caller provides the expected probability that @var{exp} == @var{c}.
> +The last argument, @var{probability}, is a floating point value in the
> +range 0.0 to 1.0, inclusive.
>  @end deftypefn

That's definitely much better, can you pleas install it?

Thanks,
Martin

>
>  @deftypefn {Built-in Function} void __builtin_trap (void)
>

Reply | Threaded
Open this post in threaded view
|

Re: [wwwdocs] Typo in description of __builtin_expect_with_probability

Jonathan Wakely-4
On Tue, 30 Oct 2018 at 12:06, Martin Liška <[hidden email]> wrote:

>
> On 10/30/18 10:58 AM, Jonathan Wakely wrote:
> > On Tue, 30 Oct 2018 at 09:54, Jonathan Wakely <[hidden email]> wrote:
> >>
> >> On Tue, 30 Oct 2018 at 09:18, Rasmus Villemoes <[hidden email]> wrote:
> >>>
> >>> On 2018-10-30 09:29, Martin Liška wrote:
> >>>> On 10/30/18 3:56 AM, Alexander Oblovatniy wrote:
> >>>>> Hello,
> >>>>>
> >>>>> I'd like to report a typo in description of
> >>>>> «__builtin_expect_with_probability»:
> >>>>> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins
> >>>>>
> >>>>> The description starts with "The built-in has same semantics as
> >>>>> *__builtin_expect_with_probability*", but it seems like *__builtin_expect*
> >>>>> should be there.
> >>>>
> >>>> Thanks for reporting the issue. It's fixed as r265615.
> >>>
> >>> Other issues around the same place: "expected probability (in percent)"
> >>> seems to contradict "valid values are in inclusive range 0.0f and 1.0f".
> >>> The prototype is listed as
> >>>
> >>> (long exp, long c, long probability)
> >>
> >> The testcases added alongside the new built-in use float arguments,
> >> but the actual definition of the built-in seems to use a long double.
> >>
> >> What does "user can provide expected probability (in percent) for
> >> value of @var{exp}" mean? What is the probability of the value of exp?
> >> Don't you mean the probability that exp==c?
> >>
> >> Also the grammar in the text needs several fixes.
> >
> > Maybe something like:
> >
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -12025,12 +12025,12 @@ when testing pointer or floating-point values.
> >  @end deftypefn
> >
> >  @deftypefn {Built-in Function} long __builtin_expect_with_probability
> > -(long @var{exp}, long @var{c}, long @var{probability})
> > +(long @var{exp}, long @var{c}, long double @var{probability})
>
> Hi.
>
> Thanks for rewording!
>
> I guess we should use:
> s/long double/double

Yup.

> >
> > -The built-in has same semantics as @code{__builtin_expect},
> > -but user can provide expected probability (in percent) for value of @var{exp}.
> > -Last argument @var{probability} is of float type and valid values
> > -are in inclusive range 0.0f and 1.0f.
> > +This function has the same semantics as @code{__builtin_expect},
> > +but the caller provides the expected probability that @var{exp} == @var{c}.
> > +The last argument, @var{probability}, is a floating point value in the
> > +range 0.0 to 1.0, inclusive.
> >  @end deftypefn
>
> That's definitely much better, can you pleas install it?

OK, will do - thanks everyone.