2014-09-11 21:39 GMT+01:00 Jeff Law <l...@redhat.com>: > > On 09/08/14 07:57, Jiong Wang wrote: >>> >>> Conceptually OK. Some questions/concerns about the implementation. >> >> Hi Jeff, >> >> thanks very much for your review. >>> >>> It seems to me that what you're trying to describe on the RHS is >>> REG_P || CONSTANT_P >> >> yes. >> >> and actually I am trying to detect all rtx which contains any number >> of RTX_CONST_OBJs and no more than one REG. >>> >>> >>> Note that CONSTANT_P will catch things like (high (symbol_ref)) and >>> (lo_sum (reg) (symbol_ref)) which are often used to build addresses on >>> some targets. >>> >>> With that in mind, rather than using a for_each_rtx callback, test >>> if (REG_P (src) || CONSTANT_P (src)) >>> >>> Note that SRC, when it is a CONSTANT_P, may have a variety of forms >>> with embedded registers. You'll need to verify that all of those >>> registers are not assigned after the insn with the CONSTANT_P source >>> operand. Right now you only perform that check when SRC is a REG_P. >> >> >> I am using the for_each_rtx because I want to scan "src" so that >> any sub-operands are checked, the number of REG and non-constant >> objects are record in "reg_found" and "nonconst_found". the embedded >> register found also record in the "reg" field of the structure >> "rtx_search_arg", >
Hi Jeff, > Constants in this context are going to satisfy CONSTANT_P, you don't need to manually verify them. It will include simple constants and constants which are built up out of multiple instructions (symbolic constants in particular). ===> thanks, will investigate this tomorrow. > > I suspect you still need the callback to verify the # of registers is just 1 > so that the later tests work. ===> the reg_found will record the # of registers, and there will be a check to make sure no register or just 1 register in the src. ===> so if "reg_found" is bigger than 1, then src will be assigned to NULL_RTX, that we just stop the move. + if (arg.nonconst_found + || arg.reg_found > 1) + src = NULL_RTX; ===> am I missing something here? >However, please don't use for_each_rtx, we're moving away from that to a more >efficient walker FOR_EACH_SUBRTX. ===> OK, will fix. Thanks very much for review. Regards, Jiong > > Jeff