Re: [PATCH] For FFS/CLZ/CTZ/CLRSB/POPCOUNT/PARITY/BSWAP require operand mode equal to operation mode (or VOIDmode) (PR middle-end/50161)

[Georg-Johann Lay <avr@gjlay.de>]

Jakub Jelinek wrote:
> On Tue, Aug 23, 2011 at 11:57:58AM +0200, Bernd Schmidt wrote:
>> On 08/23/11 11:52, Jakub Jelinek wrote:
>>> On Tue, Aug 23, 2011 at 11:35:07AM +0200, Bernd Schmidt wrote:
>>>>> cse_process_notes_1
>>>>> perhaps could be changed for VOIDmode new_rtx to try to
>>>>> simplify_replace_rtx it...
>>>> Is this where the problem came from? Sounds like it's worth a try.
>>> In this case, yes.  But there are many other places all around the
>>> compiler that need to disallow unary op with VOIDmode operand.
>>> In cse.c alone e.g. fold_rtx (twice), in combine.c e.g. in do_SUBST,
>>> subst, etc.  Do we want to special case all those 7 unary ops there too?
>>> Is it really worth it to save one subreg or truncate in the md patterns
>>> for rarely used rtxes?
>> Maybe not. I'll approve a patch to change it back, even if I think it's
>> not a good representation.
> 
> We can remove that restriction again once CONST_INTs are no longer VOIDmode.
> 
> Here is an untested patch, will bootstrap/regtest it now on x86_64-linux
> and i686-linux, on c6x it should make no difference IMHO (looked like a typo
> in the expander which wasn't used anyway), can somebody test it on AVR and

Tested you patch against r177949 on avr-unknown-none for C/C++.
There are no regressions and the new test case passes fine.

Johann

> BFIN?  My grepping through *.md didn't find any other places where the
> operand wouldn't have the same mode as operation.
> 
> 2011-08-23  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/50161
> 	* simplify-rtx.c (simplify_const_unary_operation): If
> 	op is CONST_INT, don't look at op_mode, but use instead
> 	mode.
> 	* optabs.c (add_equal_note): For FFS, CLZ, CTZ,
> 	CLRSB, POPCOUNT, PARITY and BSWAP use operand mode for
> 	operation and TRUNCATE/ZERO_EXTEND if needed.
> 	* doc/rtl.texi (ffs, clrsb, clz, ctz, popcount, parity, bswap):
> 	Document that operand mode must be same as operation mode,
> 	or VOIDmode.
> 	* config/avr/avr.md (paritysi2, *parityqihi2.libgcc,
> 	*paritysihi2.libgcc, popcountsi2, *popcountsi2.libgcc,
> 	*popcountqihi2.libgcc, clzsi2, *clzsihi2.libgcc, ctzsi2,
> 	*ctzsihi2.libgcc, ffssi2, *ffssihi2.libgcc): For unary ops
> 	use the mode of operand for the operation and add truncate
> 	or zero_extend around if needed.
> 	* config/c6x/c6x.md (ctzdi2): Likewise.
> 	* config/bfin/bfin.md (clrsbsi2, signbitssi2): Likewise.
> 
> 	* gcc.dg/pr50161.c: New test.