> From: Jeff Law [mailto:l...@redhat.com] > Sent: Friday, April 24, 2015 10:59 AM >
Hi Jeff, > > + > > +static bool > > +cprop_reg_p (const_rtx x) > > +{ > > + return REG_P (x) && !HARD_REGISTER_P (x); > > +} > How about instead this move to a more visible location (perhaps a macro > in regs.h or an inline function). Then as a followup, change the > various places that have this sequence to use that common definition > that exist outside of cprop.c. According to Steven this was proposed in the past but was refused (see end of [1]). [1] https://gcc.gnu.org/ml/gcc-patches/2015-03/msg01066.html > > > @@ -1191,7 +1192,7 @@ do_local_cprop (rtx x, rtx_insn *insn) > > /* Rule out USE instructions and ASM statements as we don't want > to > > change the hard registers mentioned. */ > > if (REG_P (x) > > - && (REGNO (x) >= FIRST_PSEUDO_REGISTER > > + && (cprop_reg_p (x) > > || (GET_CODE (PATTERN (insn)) != USE > > && asm_noperands (PATTERN (insn)) < 0))) > Isn't the REG_P test now redundant? I made the same mistake when reviewing that change and indeed it's not. Note the opening parenthesis before cprop_reg_p that contains a bitwise OR expression. So in the case where cprop_reg_p is false, REG_P still needs to be true. We could keep a check on FIRST_PSEUDO_REGISTER but the intent (checking that the register is suitable for propagation) is clearer now, as pointed out by Steven to me. > > OK for the trunk with those changes. > > jeff Given the above I intent to keep the REG_P in the second excerpt and will wait for your input about moving cprop_reg_p to rtl.h Best regards, Thomas