Message ID | 5064DA18.20404@redhat.com |
---|---|
State | New |
Headers | show |
On 09/27/2012 04:58 PM, Vladimir Makarov wrote: > The following patch modifies some code in the rest of compiler for > correct work of LRA. The code works the same way when LRA is not > used. It is achieved by checking a new variable lra_in_progress. > > 2012-09-27 Vladimir Makarov <vmakarov@redhat.com> > > * rtlanal.c (simplify_subreg_regno): Permit ARG_POINTER_REGNUM and > STACK_POINTER_REGNU for LRA. > * jump.c (true_regnum): Always use hard_regno for subreg_get_info when > lra is in progress. > * expr.c (emit_move_insn_1): Pass an additional argument to > emit_move_via_integer. Use emit_move_via_integer for LRA only if > the insn is recognized. > * recog.c (general_operand, register_operand): Accept paradoxical > FLOAD_MODE > subregs for LRA. > (scratch_operand): Accept pseudos for LRA. > * emit-rtl.c (gen_rtx_REG): Add lra_in_progress. > (validate_subreg): Don't check offset for LRA and > floating point modes. > * rtl.h (lra_in_progress): New external. > * ira.c (lra_in_progress): Define. s/FLOAD/FLOAT/ to fix ChangeLog typo. > Index: jump.c > =================================================================== > --- jump.c (revision 191771) > +++ jump.c (working copy) > @@ -1868,7 +1868,8 @@ true_regnum (const_rtx x) > { > if (REG_P (x)) > { > - if (REGNO (x) >= FIRST_PSEUDO_REGISTER && reg_renumber[REGNO (x)] >= 0) > + if (REGNO (x) >= FIRST_PSEUDO_REGISTER > + && (lra_in_progress || reg_renumber[REGNO (x)] >= 0)) > return reg_renumber[REGNO (x)]; > return REGNO (x); > } This hunk doesn't make any sense to me, unless you want true_regnum to return a negative value during LRA for cases where the pseudo is still unassigned. Is that what's you're intending here? If that's what you want, then I think it's worth a quick comment. > @@ -1880,7 +1881,8 @@ true_regnum (const_rtx x) > { > struct subreg_info info; > > - subreg_get_info (REGNO (SUBREG_REG (x)), > + subreg_get_info (lra_in_progress > + ? (unsigned) base : REGNO (SUBREG_REG (x)), > GET_MODE (SUBREG_REG (x)), > SUBREG_BYTE (x), GET_MODE (x), &info); I'd be good to indicate why you want to do something different for LRA here. > > Index: rtlanal.c > =================================================================== > --- rtlanal.c (revision 191771) > +++ rtlanal.c (working copy) > @@ -3465,7 +3465,9 @@ simplify_subreg_regno (unsigned int xreg > /* Give the backend a chance to disallow the mode change. */ > if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT > && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT > - && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode)) > + && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode) > + /* We can use mode change in LRA for some transformations. */ > + && ! lra_in_progress) > return -1; > #endif I don't think this change is reflected in the ChangeLog. I think just the minor ChangeLog updates and clarification of the changes to jump.c are all that's needed for this patch to be approved. jeff
On 12-09-28 4:07 PM, Jeff Law wrote: > On 09/27/2012 04:58 PM, Vladimir Makarov wrote: >> The following patch modifies some code in the rest of compiler for >> correct work of LRA. The code works the same way when LRA is not >> used. It is achieved by checking a new variable lra_in_progress. >> >> 2012-09-27 Vladimir Makarov <vmakarov@redhat.com> >> >> * rtlanal.c (simplify_subreg_regno): Permit ARG_POINTER_REGNUM and >> STACK_POINTER_REGNU for LRA. >> * jump.c (true_regnum): Always use hard_regno for >> subreg_get_info when >> lra is in progress. >> * expr.c (emit_move_insn_1): Pass an additional argument to >> emit_move_via_integer. Use emit_move_via_integer for LRA only if >> the insn is recognized. >> * recog.c (general_operand, register_operand): Accept paradoxical >> FLOAD_MODE >> subregs for LRA. >> (scratch_operand): Accept pseudos for LRA. >> * emit-rtl.c (gen_rtx_REG): Add lra_in_progress. >> (validate_subreg): Don't check offset for LRA and >> floating point modes. >> * rtl.h (lra_in_progress): New external. >> * ira.c (lra_in_progress): Define. > s/FLOAD/FLOAT/ to fix ChangeLog typo. > > Thanks. I fixed it will be in the revised versions of the patches. > >> Index: jump.c >> =================================================================== >> --- jump.c (revision 191771) >> +++ jump.c (working copy) >> @@ -1868,7 +1868,8 @@ true_regnum (const_rtx x) >> { >> if (REG_P (x)) >> { >> - if (REGNO (x) >= FIRST_PSEUDO_REGISTER && reg_renumber[REGNO >> (x)] >= 0) >> + if (REGNO (x) >= FIRST_PSEUDO_REGISTER >> + && (lra_in_progress || reg_renumber[REGNO (x)] >= 0)) >> return reg_renumber[REGNO (x)]; >> return REGNO (x); >> } > This hunk doesn't make any sense to me, unless you want true_regnum to > return a negative value during LRA for cases where the pseudo is still > unassigned. Is that what's you're intending here? If that's what you > want, then I think it's worth a quick comment. > Yes, that was my intention. LRA works a bit different from reload. It needs the current assignment of pseudos (assigned hard register or -1 even it is not assigned). I'll add the comment about this in the next version of the patch. The code looks a bit strange and could be more clear but I kept in my mind removing code for reload in the future. > > >> @@ -1880,7 +1881,8 @@ true_regnum (const_rtx x) >> { >> struct subreg_info info; >> >> - subreg_get_info (REGNO (SUBREG_REG (x)), >> + subreg_get_info (lra_in_progress >> + ? (unsigned) base : REGNO (SUBREG_REG (x)), >> GET_MODE (SUBREG_REG (x)), >> SUBREG_BYTE (x), GET_MODE (x), &info); > I'd be good to indicate why you want to do something different for LRA > here. > I need to know the current allocation with taking into account that the subregister with final hard register will be representable. I'll add the comment. >> >> Index: rtlanal.c >> =================================================================== >> --- rtlanal.c (revision 191771) >> +++ rtlanal.c (working copy) >> @@ -3465,7 +3465,9 @@ simplify_subreg_regno (unsigned int xreg >> /* Give the backend a chance to disallow the mode change. */ >> if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT >> && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT >> - && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode)) >> + && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode) >> + /* We can use mode change in LRA for some transformations. */ >> + && ! lra_in_progress) >> return -1; >> #endif > I don't think this change is reflected in the ChangeLog. > You are right. I added the change description in the ChangeLog. Reload has no problem in representation of some decisions because it uses internal representation. It is a problem for LRA using RTL when for example two insn operands in different modes should have the same hard register according to the constraint. I use subreg for that even it is not a correct in other parts of the compiler. > I think just the minor ChangeLog updates and clarification of the > changes to jump.c are all that's needed for this patch to be approved. > Thanks, Jeff. I really appreciate your help.
Index: rtl.h =================================================================== --- rtl.h (revision 191771) +++ rtl.h (working copy) @@ -2369,6 +2369,9 @@ extern int epilogue_completed; extern int reload_in_progress; +/* Set to 1 while in lra. */ +extern int lra_in_progress; + /* This macro indicates whether you may create a new pseudo-register. */ Index: ira.c =================================================================== --- ira.c (revision 191771) +++ ira.c (working copy) @@ -4308,6 +4308,9 @@ bool ira_conflicts_p; /* Saved between IRA and reload. */ static int saved_flag_ira_share_spill_slots; +/* Set to 1 while in lra. */ +int lra_in_progress = 0; + /* This is the main entry of IRA. */ static void ira (FILE *f) Index: jump.c =================================================================== --- jump.c (revision 191771) +++ jump.c (working copy) @@ -1868,7 +1868,8 @@ true_regnum (const_rtx x) { if (REG_P (x)) { - if (REGNO (x) >= FIRST_PSEUDO_REGISTER && reg_renumber[REGNO (x)] >= 0) + if (REGNO (x) >= FIRST_PSEUDO_REGISTER + && (lra_in_progress || reg_renumber[REGNO (x)] >= 0)) return reg_renumber[REGNO (x)]; return REGNO (x); } @@ -1880,7 +1881,8 @@ true_regnum (const_rtx x) { struct subreg_info info; - subreg_get_info (REGNO (SUBREG_REG (x)), + subreg_get_info (lra_in_progress + ? (unsigned) base : REGNO (SUBREG_REG (x)), GET_MODE (SUBREG_REG (x)), SUBREG_BYTE (x), GET_MODE (x), &info); Index: rtlanal.c =================================================================== --- rtlanal.c (revision 191771) +++ rtlanal.c (working copy) @@ -3465,7 +3465,9 @@ simplify_subreg_regno (unsigned int xreg /* Give the backend a chance to disallow the mode change. */ if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT - && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode)) + && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode) + /* We can use mode change in LRA for some transformations. */ + && ! lra_in_progress) return -1; #endif @@ -3475,10 +3477,16 @@ simplify_subreg_regno (unsigned int xreg return -1; if (FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM - && xregno == ARG_POINTER_REGNUM) + /* We should convert arg register in LRA after the elimination + if it is possible. */ + && xregno == ARG_POINTER_REGNUM + && ! lra_in_progress) return -1; - if (xregno == STACK_POINTER_REGNUM) + if (xregno == STACK_POINTER_REGNUM + /* We should convert hard stack register in LRA if it is + possible. */ + && ! lra_in_progress) return -1; /* Try to get the register offset. */ Index: emit-rtl.c =================================================================== --- emit-rtl.c (revision 191771) +++ emit-rtl.c (working copy) @@ -581,7 +581,7 @@ gen_rtx_REG (enum machine_mode mode, uns Also don't do this when we are making new REGs in reload, since we don't want to get confused with the real pointers. */ - if (mode == Pmode && !reload_in_progress) + if (mode == Pmode && !reload_in_progress && !lra_in_progress) { if (regno == FRAME_POINTER_REGNUM && (!reload_completed || frame_pointer_needed)) @@ -723,7 +723,14 @@ validate_subreg (enum machine_mode omode (subreg:SI (reg:DF) 0) isn't. */ else if (FLOAT_MODE_P (imode) || FLOAT_MODE_P (omode)) { - if (isize != osize) + if (! (isize == osize + /* LRA can use subreg to store a floating point value in + an integer mode. Although the floating point and the + integer modes need the same number of hard registers, + the size of floating point mode can be less than the + integer mode. LRA also uses subregs for a register + should be used in different mode in on insn. */ + || lra_in_progress)) return false; } @@ -756,7 +763,8 @@ validate_subreg (enum machine_mode omode of a subword. A subreg does *not* perform arbitrary bit extraction. Given that we've already checked mode/offset alignment, we only have to check subword subregs here. */ - if (osize < UNITS_PER_WORD) + if (osize < UNITS_PER_WORD + && ! (lra_in_progress && (FLOAT_MODE_P (imode) || FLOAT_MODE_P (omode)))) { enum machine_mode wmode = isize > UNITS_PER_WORD ? word_mode : imode; unsigned int low_off = subreg_lowpart_offset (omode, wmode); Index: expr.c =================================================================== --- expr.c (revision 191771) +++ expr.c (working copy) @@ -3434,9 +3434,13 @@ emit_move_insn_1 (rtx x, rtx y) fits within a HOST_WIDE_INT. */ if (!CONSTANT_P (y) || GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT) { - rtx ret = emit_move_via_integer (mode, x, y, false); + rtx ret = emit_move_via_integer (mode, x, y, lra_in_progress); + if (ret) - return ret; + { + if (! lra_in_progress || recog (PATTERN (ret), ret, 0) >= 0) + return ret; + } } return emit_move_multi_word (mode, x, y); Index: recog.c =================================================================== --- recog.c (revision 191771) +++ recog.c (working copy) @@ -993,6 +993,12 @@ general_operand (rtx op, enum machine_mo /* FLOAT_MODE subregs can't be paradoxical. Combine will occasionally create such rtl, and we must reject it. */ if (SCALAR_FLOAT_MODE_P (GET_MODE (op)) + /* LRA can use subreg to store a floating point value in an + integer mode. Although the floating point and the + integer modes need the same number of hard registers, the + size of floating point mode can be less than the integer + mode. */ + && ! lra_in_progress && GET_MODE_SIZE (GET_MODE (op)) > GET_MODE_SIZE (GET_MODE (sub))) return 0; @@ -1068,6 +1074,12 @@ register_operand (rtx op, enum machine_m /* FLOAT_MODE subregs can't be paradoxical. Combine will occasionally create such rtl, and we must reject it. */ if (SCALAR_FLOAT_MODE_P (GET_MODE (op)) + /* LRA can use subreg to store a floating point value in an + integer mode. Although the floating point and the + integer modes need the same number of hard registers, the + size of floating point mode can be less than the integer + mode. */ + && ! lra_in_progress && GET_MODE_SIZE (GET_MODE (op)) > GET_MODE_SIZE (GET_MODE (sub))) return 0; @@ -1099,7 +1111,7 @@ scratch_operand (rtx op, enum machine_mo return (GET_CODE (op) == SCRATCH || (REG_P (op) - && REGNO (op) < FIRST_PSEUDO_REGISTER)); + && (lra_in_progress || REGNO (op) < FIRST_PSEUDO_REGISTER))); } /* Return 1 if OP is a valid immediate operand for mode MODE.