On Tue, Aug 23, 2011 at 11:35 AM, Bernd Schmidt <ber...@codesourcery.com> wrote: > On 08/23/11 11:05, Jakub Jelinek wrote: >> On Mon, Jun 20, 2011 at 09:38:22PM +0200, Bernd Schmidt wrote: >>> D'oh. Blackfin has a (clrsb:HI (operand:SI)) instruction, so adding this >>> showed a problem with some of the existing simplify_const_unop cases: >>> for ffs/clz/ctz/clrsb/parity/popcount, we should look at the mode of the >>> operand, rather than the mode of the operation. This limits what we can >>> do in that function, since op_mode is sometimes VOIDmode - we really >>> should add builtin folders for these at some point. >> >>> * simplify-rtx.c (simplify_const_unary_operation): Likewise. >>> Use op_mode rather than mode when optimizing ffs, clz, ctz, parity >>> and popcount. >> >> This change is IMHO wrong, > > Conceptually, I think it is exactly right. It may however be > inconvenient in some cases. > >> see e.g. >> PR50161 where we have (subreg:SI (popcount:DI (const_int -1))). This >> is supposed to yield 64, but with your changes >> it yields 128 - the op_mode here is VOIDmode, > > This is what shouldn't happen.
If it shouldn't happen, does some verifier catch it? >> 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. > > Wasn't Richard S. working on a patch to give constants modes? > > > Bernd > >
