Re: [PATCH] Fix RTL simplifications of FFS, POPCOUNT and PARITY.

[Jeff Law <jeffreyalaw@gmail.com>]

On 1/2/23 10:30, roger@nextmovesoftware.com wrote:
> 
> Hi Jeff,
> 
>> On 2 Jan 2023, at 15:45, Jeff Law <jeffreyalaw@gmail.com> wrote:
>> On 1/1/23 08:55, Roger Sayle wrote:
>>> In 2011, the rtl.texi documentation was updated to reflect that the
>>> modes of the RTX unary operations FFS, POPCOUNT and PARITY must
>>> match those of their operands.  Unfortunately, some of the transformations
>>> in simplify-rtx.cc predate this tightening of RTL semantics, and have
>>> not (until now) been updated/fixed.  i.e. The POPCOUNT and PARITY
>>> optimizations were "correct" when I originally added them back in 2007.
>>> Segher requested that I split this piece out from a fix for PR 106594 in
>>> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601501.html
>>> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
>>> and make -k check, both with and without --target_board=unix{-m32},
>>> with no new failures.  Ok for mainline?
>>> 2023-01-01  Roger Sayle  <roger@nextmovesoftware.com>
>>> gcc/ChangeLog
>>> 	* gcc/simplify-rtx.cc (simplify_unary_operation_1) <case FFS>:
>>> 	Avoid generating FFS with mismatched operand and result modes, by
>>> 	using an explicit SIGN_EXTEND/ZERO_EXTEND instead.
>>> 	<case POPCOUNT>: Likewise, for POPCOUNT of ZERO_EXTEND.
>>> 	<case PARITY>: Likewise, for PARITY of {ZERO,SIGN}_EXTEND.
>> ?!?  The docs still seem to indicate to me that the modes of the input and output operands can differ.
>> Let's take PARITY as an example:
>>
>>> @cindex @code{parity@var{m}2} instruction pattern
>>> @item @samp{parity@var{m}2}
>>> Store into operand 0 the parity of operand 1, i.e.@: the number of 1-bits
>>> in operand 1 modulo 2.
>>> @var{m} is either a scalar or vector integer mode.  When it is a scalar,
>>> operand 1 has mode @var{m} but operand 0 can have whatever scalar
>>> integer mode is suitable for the target.
>>
>> The mode of the pattern name has to match the mode of the input operand.  The mode of the
>> output operand can differ from the mode of the input operand.  we seem to have a disagreement
>> on the documented semantics of these opcodes.
> 
> The documentation that you're looking at is the definition of the parity optab in
> md.texi, not the definition of the PARITY rtx in rtl.texi.  The distinction is subtle.
> Hence a backend can define paritysiqi2 but in the RTL pattern it expands to the
> unary PARITY operator must have the same result type as its operand type,
> wrapped in either a truncate or extension if necessary.
Hmm, yea I was looking at md.texi, not rtl.texi.  I'll take a look at 
the latter after I get some coffee.

Thanks!
jeff