| Message ID | ceb84686-9cdf-d582-bb9e-e3d828c54bb6@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [LRA] Fix PR88033: ICE in remove_some_program_points_and_update_live_ranges | expand |
On 11/16/18 1:15 PM, Peter Bergner wrote: > This is currently bootstrapping and regtesting on x86_64-linux. > Ok for mainline assuming the tests show no regressions? > > gcc/ > PR rtl-optimization/88033 > * ira-lives.c (non_conflicting_reg_copy_p): Skip copies from a register > to itself. Use HARD_REGISTER_NUM_P. > > gcc/testsuite/ > PR rtl-optimization/88033 > * gcc.target/i386/pr88033.c: New test. FYI, testing completed with no regressions. Peter
On 11/16/2018 02:15 PM, Peter Bergner wrote: > PR88033 shows a problem when handling simple copies from a register to itself: > > (insn (set (reg:DI NNN) (reg:DI NNN))) > > This was causing confusion in the code that performs liveness and conflict > updates and program point updates in lra-lives.c. Trying to handle these > types of copies would add some ugly code. It's easier to just bail on them > in non_conflicting_reg_copy_p() altogether, since by definition, a register > does not conflict with itself and so needs no special handling. The patch > below implements that and fixes the ICE. > > This is currently bootstrapping and regtesting on x86_64-linux. > Ok for mainline assuming the tests show no regressions? > OK. Thank you, Peter. > > > gcc/ > PR rtl-optimization/88033 > * ira-lives.c (non_conflicting_reg_copy_p): Skip copies from a register > to itself. Use HARD_REGISTER_NUM_P. > > gcc/testsuite/ > PR rtl-optimization/88033 > * gcc.target/i386/pr88033.c: New test. > > > Index: gcc/ira-lives.c > =================================================================== > --- gcc/ira-lives.c (revision 266207) > +++ gcc/ira-lives.c (working copy) > @@ -1083,11 +1083,17 @@ non_conflicting_reg_copy_p (rtx_insn *insn) > int src_regno = REGNO (SET_SRC (set)); > machine_mode mode = GET_MODE (SET_DEST (set)); > > + /* By definition, a register does not conflict with itself, therefore we > + do not have to handle it specially. Returning NULL_RTX now, helps > + simplify the callers of this function. */ > + if (dst_regno == src_regno) > + return NULL_RTX; > + > /* Computing conflicts for register pairs is difficult to get right, so > for now, disallow it. */ > - if ((dst_regno < FIRST_PSEUDO_REGISTER > + if ((HARD_REGISTER_NUM_P (dst_regno) > && hard_regno_nregs (dst_regno, mode) != 1) > - || (src_regno < FIRST_PSEUDO_REGISTER > + || (HARD_REGISTER_NUM_P (src_regno) > && hard_regno_nregs (src_regno, mode) != 1)) > return NULL_RTX; > > Index: gcc/testsuite/gcc.target/i386/pr88033.c > =================================================================== > --- gcc/testsuite/gcc.target/i386/pr88033.c (nonexistent) > +++ gcc/testsuite/gcc.target/i386/pr88033.c (working copy) > @@ -0,0 +1,14 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +int > +main (long a, long *b, long c) > +{ > + if (!c) > + return 0; > + int g; > + *b = (g & ~3000000000) < 0 ? a : a - g; > + while (1) > + ; > + return 0; > +} >
On 11/19/18 1:17 PM, Vladimir Makarov wrote: > On 11/16/2018 02:15 PM, Peter Bergner wrote: >> PR88033 shows a problem when handling simple copies from a register to itself: >> >> (insn (set (reg:DI NNN) (reg:DI NNN))) >> >> This was causing confusion in the code that performs liveness and conflict >> updates and program point updates in lra-lives.c. Trying to handle these >> types of copies would add some ugly code. It's easier to just bail on them >> in non_conflicting_reg_copy_p() altogether, since by definition, a register >> does not conflict with itself and so needs no special handling. The patch >> below implements that and fixes the ICE. >> >> This is currently bootstrapping and regtesting on x86_64-linux. >> Ok for mainline assuming the tests show no regressions? >> > OK. Thank you, Peter. Thanks, this is now committed. Peter
Index: gcc/ira-lives.c =================================================================== --- gcc/ira-lives.c (revision 266207) +++ gcc/ira-lives.c (working copy) @@ -1083,11 +1083,17 @@ non_conflicting_reg_copy_p (rtx_insn *insn) int src_regno = REGNO (SET_SRC (set)); machine_mode mode = GET_MODE (SET_DEST (set)); + /* By definition, a register does not conflict with itself, therefore we + do not have to handle it specially. Returning NULL_RTX now, helps + simplify the callers of this function. */ + if (dst_regno == src_regno) + return NULL_RTX; + /* Computing conflicts for register pairs is difficult to get right, so for now, disallow it. */ - if ((dst_regno < FIRST_PSEUDO_REGISTER + if ((HARD_REGISTER_NUM_P (dst_regno) && hard_regno_nregs (dst_regno, mode) != 1) - || (src_regno < FIRST_PSEUDO_REGISTER + || (HARD_REGISTER_NUM_P (src_regno) && hard_regno_nregs (src_regno, mode) != 1)) return NULL_RTX; Index: gcc/testsuite/gcc.target/i386/pr88033.c =================================================================== --- gcc/testsuite/gcc.target/i386/pr88033.c (nonexistent) +++ gcc/testsuite/gcc.target/i386/pr88033.c (working copy) @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +int +main (long a, long *b, long c) +{ + if (!c) + return 0; + int g; + *b = (g & ~3000000000) < 0 ? a : a - g; + while (1) + ; + return 0; +}