diff mbox

[doc] Document clrsb optab and fix some inconsistencies

Message ID 53C64A00.7080303@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov July 16, 2014, 9:46 a.m. UTC 
On 16/07/14 09:25, James Greenhalgh wrote:
> On Wed, Jul 16, 2014 at 09:05:22AM +0100, Kyrill Tkachov wrote:
>> Hi all,
>>
>> I noticed we don't document the clrsb<mode>2 optab but it does exist.
>> The proposed description is based on the clrsb RTL code documentation in
>> rtl.texi.
>> While we're at it, clean up the wording for some other cases in the file
>> which refer to an argument 'x' when they mean to refer to operand 1.
>> They were probably copied over from rtl.texi which does use 'x'.
>>
>> Ok for trunk?
>>
>> 2014-07-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>       * doc/md.texi (clrsb): Document.
>>       (clz): Change reference to x into operand 1.
>>       (ctz): Likewise.
>>       (popcount): Likewise.
>>       (parity): Likewise.
>> commit 8f0f09fbf6e8ee8136d4977f9819ee1997f82963
>> Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>> Date:   Tue Jul 15 17:41:29 2014 +0100
>>
>>      [DOC] Document clrsb optab
>>
>> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
>> index fde67d7..268f625 100644
>> --- a/gcc/doc/md.texi
>> +++ b/gcc/doc/md.texi
>> @@ -5316,10 +5316,17 @@ generating the instruction.
>>   The @code{ffs} built-in function of C always uses the mode which
>>   corresponds to the C data type @code{int}.
>>   
>> +@cindex @code{clrsb@var{m}2} instruction pattern
>> +@item @samp{clrsb@var{m}2}
>> +Store into operand 0 the number of redundant sign bits in operand 1, starting
>> +at the most significant bit position.
>> +This is one less than the number of leading sign bits (either 0 or 1),
>> +with no special cases.
>> +
> This reads unclear to me. If I have no knowledge of clrsb the sentence:
>
>    "This is one less than the number of leading sign bits (either 0 or 1)"
>
> can be read as suggesting that there are either 0 or 1 leading sign bits,
> with clrsb setting operand 0 to be one less than that (either -1 or 0).
>
> I think giving the example of what "sign bits" are is unnecessary, likewise
> the "with no special cases" clause. The only term which might need introducing
> is what a redundant sign bit is. I also find it polite to expand the acronym
> shortly after first use. I think this hunk would read clearer as:
>
>    "Count leading redundant sign bits.
>
>     Store into operand 0 the number of redundant sign bits in operand 1,
>     starting at the most significant bit position.
>
>     A redundant sign bit is defined as any sign bit after the first. As such,
>     this count will be one less than the count of leading sign bits."
Hi James,

You're right, the text is somewhat ambiguous, here is the patch with 
your suggestion.
Any of the two versions can go in I guess, although this second one is 
clearer IMHO.

Thanks,
Kyrill

2014-07-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
             James Greenhalgh  <james.greenhalgh@arm.com>

      * doc/md.texi (clrsb): Document.
      (clz): Change reference to x into operand 1.
      (ctz): Likewise.
      (popcount): Likewise.
      (parity): Likewise.

Comments

Kyrylo Tkachov July 23, 2014, 8:54 a.m. UTC | #1 
Ping.

On 16/07/14 10:46, Kyrill Tkachov wrote:
> On 16/07/14 09:25, James Greenhalgh wrote:
>> On Wed, Jul 16, 2014 at 09:05:22AM +0100, Kyrill Tkachov wrote:
>>> Hi all,
>>>
>>> I noticed we don't document the clrsb<mode>2 optab but it does exist.
>>> The proposed description is based on the clrsb RTL code documentation in
>>> rtl.texi.
>>> While we're at it, clean up the wording for some other cases in the file
>>> which refer to an argument 'x' when they mean to refer to operand 1.
>>> They were probably copied over from rtl.texi which does use 'x'.
>>>
>>> Ok for trunk?
>>>
>>> 2014-07-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>        * doc/md.texi (clrsb): Document.
>>>        (clz): Change reference to x into operand 1.
>>>        (ctz): Likewise.
>>>        (popcount): Likewise.
>>>        (parity): Likewise.
>>> commit 8f0f09fbf6e8ee8136d4977f9819ee1997f82963
>>> Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>> Date:   Tue Jul 15 17:41:29 2014 +0100
>>>
>>>       [DOC] Document clrsb optab
>>>
>>> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
>>> index fde67d7..268f625 100644
>>> --- a/gcc/doc/md.texi
>>> +++ b/gcc/doc/md.texi
>>> @@ -5316,10 +5316,17 @@ generating the instruction.
>>>    The @code{ffs} built-in function of C always uses the mode which
>>>    corresponds to the C data type @code{int}.
>>>    
>>> +@cindex @code{clrsb@var{m}2} instruction pattern
>>> +@item @samp{clrsb@var{m}2}
>>> +Store into operand 0 the number of redundant sign bits in operand 1, starting
>>> +at the most significant bit position.
>>> +This is one less than the number of leading sign bits (either 0 or 1),
>>> +with no special cases.
>>> +
>> This reads unclear to me. If I have no knowledge of clrsb the sentence:
>>
>>     "This is one less than the number of leading sign bits (either 0 or 1)"
>>
>> can be read as suggesting that there are either 0 or 1 leading sign bits,
>> with clrsb setting operand 0 to be one less than that (either -1 or 0).
>>
>> I think giving the example of what "sign bits" are is unnecessary, likewise
>> the "with no special cases" clause. The only term which might need introducing
>> is what a redundant sign bit is. I also find it polite to expand the acronym
>> shortly after first use. I think this hunk would read clearer as:
>>
>>     "Count leading redundant sign bits.
>>
>>      Store into operand 0 the number of redundant sign bits in operand 1,
>>      starting at the most significant bit position.
>>
>>      A redundant sign bit is defined as any sign bit after the first. As such,
>>      this count will be one less than the count of leading sign bits."
> Hi James,
>
> You're right, the text is somewhat ambiguous, here is the patch with
> your suggestion.
> Any of the two versions can go in I guess, although this second one is
> clearer IMHO.
>
> Thanks,
> Kyrill
>
> 2014-07-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>               James Greenhalgh  <james.greenhalgh@arm.com>
>
>        * doc/md.texi (clrsb): Document.
>        (clz): Change reference to x into operand 1.
>        (ctz): Likewise.
>        (popcount): Likewise.
>        (parity): Likewise.
Jeff Law July 25, 2014, 10 p.m. UTC | #2 
On 07/23/14 02:54, Kyrill Tkachov wrote:
>>
>> 2014-07-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>               James Greenhalgh  <james.greenhalgh@arm.com>
>>
>>        * doc/md.texi (clrsb): Document.
>>        (clz): Change reference to x into operand 1.
>>        (ctz): Likewise.
>>        (popcount): Likewise.
>>        (parity): Likewise.
The second version seems better to me as well.  Particularly if I try 
and ignore that I'm already familiar with a redundant sign bit by way of 
hacking combine through the years which uses that terminology IIRC.

Ok for the trunk.  Thanks,

jeff
diff mbox

Patch

commit 57d892f820a8f3b233c087862fad0887cee06127
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Tue Jul 15 17:41:29 2014 +0100

    [DOC] Document clrsb optab

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index fde67d7..dd78611 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5316,10 +5316,18 @@  generating the instruction.
 The @code{ffs} built-in function of C always uses the mode which
 corresponds to the C data type @code{int}.
 
+@cindex @code{clrsb@var{m}2} instruction pattern
+@item @samp{clrsb@var{m}2}
+Count leading redundant sign bits.
+Store into operand 0 the number of redundant sign bits in operand 1, starting
+at the most significant bit position.
+A redundant sign bit is defined as any sign bit after the first. As such,
+this count will be one less than the count of leading sign bits.
+
 @cindex @code{clz@var{m}2} instruction pattern
 @item @samp{clz@var{m}2}
-Store into operand 0 the number of leading 0-bits in @var{x}, starting
-at the most significant bit position.  If @var{x} is 0, the
+Store into operand 0 the number of leading 0-bits in operand 1, starting
+at the most significant bit position.  If operand 1 is 0, the
 @code{CLZ_DEFINED_VALUE_AT_ZERO} (@pxref{Misc}) macro defines if
 the result is undefined or has a useful value.
 @var{m} is the mode of operand 0; operand 1's mode is
@@ -5328,8 +5336,8 @@  operand to that mode before generating the instruction.
 
 @cindex @code{ctz@var{m}2} instruction pattern
 @item @samp{ctz@var{m}2}
-Store into operand 0 the number of trailing 0-bits in @var{x}, starting
-at the least significant bit position.  If @var{x} is 0, the
+Store into operand 0 the number of trailing 0-bits in operand 1, starting
+at the least significant bit position.  If operand 1 is 0, the
 @code{CTZ_DEFINED_VALUE_AT_ZERO} (@pxref{Misc}) macro defines if
 the result is undefined or has a useful value.
 @var{m} is the mode of operand 0; operand 1's mode is
@@ -5338,15 +5346,15 @@  operand to that mode before generating the instruction.
 
 @cindex @code{popcount@var{m}2} instruction pattern
 @item @samp{popcount@var{m}2}
-Store into operand 0 the number of 1-bits in @var{x}.  @var{m} is the
+Store into operand 0 the number of 1-bits in operand 1.  @var{m} is the
 mode of operand 0; operand 1's mode is specified by the instruction
 pattern, and the compiler will convert the operand to that mode before
 generating the instruction.
 
 @cindex @code{parity@var{m}2} instruction pattern
 @item @samp{parity@var{m}2}
-Store into operand 0 the parity of @var{x}, i.e.@: the number of 1-bits
-in @var{x} modulo 2.  @var{m} is the mode of operand 0; operand 1's mode
+Store into operand 0 the parity of operand 1, i.e.@: the number of 1-bits
+in operand 1 modulo 2.  @var{m} is the mode of operand 0; operand 1's mode
 is specified by the instruction pattern, and the compiler will convert
 the operand to that mode before generating the instruction.