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. > 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
