| Message ID | 50C25AB6.1000807@redhat.com |
|---|---|
| State | New |
| Headers | show |
Vladimir Makarov <vmakarov@redhat.com> writes: > Index: lra-constraints.c > =================================================================== > --- lra-constraints.c (revision 194307) > +++ lra-constraints.c (working copy) > @@ -3329,8 +3329,9 @@ lra_constraints (bool first_p) > reg = regno_reg_rtx[i]; > if ((hard_regno = lra_get_regno_hard_regno (i)) >= 0) > { > - int j, nregs = hard_regno_nregs[hard_regno][PSEUDO_REGNO_MODE (i)]; > + int j, nregs; > > + nregs = hard_regno_nregs[hard_regno][lra_reg_info[i].biggest_mode]; > for (j = 0; j < nregs; j++) > df_set_regs_ever_live (hard_regno + j, true); It looks like this loop now iterates over a different mode from the pseudo register's, but starts at the hard register allocated to the pseudo. That doesn't work on big-endian targets, where the "extra" registers come before hard_regno. I think you need to use simplify_subreg_regno (...subreg_lowpart_offset (...)). I realise we only support little-endian for 4.8, but still. Maybe it'd be worth having a helper function that provides the range. Richard
On 12/08/2012 05:30 AM, Richard Sandiford wrote: > Vladimir Makarov <vmakarov@redhat.com> writes: >> Index: lra-constraints.c >> =================================================================== >> --- lra-constraints.c (revision 194307) >> +++ lra-constraints.c (working copy) >> @@ -3329,8 +3329,9 @@ lra_constraints (bool first_p) >> reg = regno_reg_rtx[i]; >> if ((hard_regno = lra_get_regno_hard_regno (i)) >= 0) >> { >> - int j, nregs = hard_regno_nregs[hard_regno][PSEUDO_REGNO_MODE (i)]; >> + int j, nregs; >> >> + nregs = hard_regno_nregs[hard_regno][lra_reg_info[i].biggest_mode]; >> for (j = 0; j < nregs; j++) >> df_set_regs_ever_live (hard_regno + j, true); > It looks like this loop now iterates over a different mode from the > pseudo register's, but starts at the hard register allocated to the > pseudo. That doesn't work on big-endian targets, where the "extra" > registers come before hard_regno. I think you need to use > simplify_subreg_regno (...subreg_lowpart_offset (...)). > I realise we only support little-endian for 4.8, but still. Yes, Richard, I am aware about it. There are a few places in LRA where similar code should be fixed for big-endian targets. I'd like to address this problem on lra-branch when I am less busy. > Maybe it'd be worth having a helper function that provides the range. >
Vladimir Makarov <vmakarov@redhat.com> writes: > On 12/08/2012 05:30 AM, Richard Sandiford wrote: >> Vladimir Makarov <vmakarov@redhat.com> writes: >>> Index: lra-constraints.c >>> =================================================================== >>> --- lra-constraints.c (revision 194307) >>> +++ lra-constraints.c (working copy) >>> @@ -3329,8 +3329,9 @@ lra_constraints (bool first_p) >>> reg = regno_reg_rtx[i]; >>> if ((hard_regno = lra_get_regno_hard_regno (i)) >= 0) >>> { >>> - int j, nregs = hard_regno_nregs[hard_regno][PSEUDO_REGNO_MODE (i)]; >>> + int j, nregs; >>> >>> + nregs = hard_regno_nregs[hard_regno][lra_reg_info[i].biggest_mode]; >>> for (j = 0; j < nregs; j++) >>> df_set_regs_ever_live (hard_regno + j, true); >> It looks like this loop now iterates over a different mode from the >> pseudo register's, but starts at the hard register allocated to the >> pseudo. That doesn't work on big-endian targets, where the "extra" >> registers come before hard_regno. I think you need to use >> simplify_subreg_regno (...subreg_lowpart_offset (...)). >> I realise we only support little-endian for 4.8, but still. > Yes, Richard, I am aware about it. There are a few places in LRA where > similar code should be fixed for big-endian targets. I'd like to > address this problem on lra-branch when I am less busy. Well, OK, but in the meantime how about putting asserts like: gcc_assert (!BYTES_BIG_ENDIAN) in the affected places? I'm just worried that this kind of thing leads to subtle wrong-code bugs in hard-to-reduce cases, so an assert would make it easier to track which places need updating. Richard
Index: lra-constraints.c =================================================================== --- lra-constraints.c (revision 194307) +++ lra-constraints.c (working copy) @@ -3329,8 +3329,9 @@ lra_constraints (bool first_p) reg = regno_reg_rtx[i]; if ((hard_regno = lra_get_regno_hard_regno (i)) >= 0) { - int j, nregs = hard_regno_nregs[hard_regno][PSEUDO_REGNO_MODE (i)]; + int j, nregs; + nregs = hard_regno_nregs[hard_regno][lra_reg_info[i].biggest_mode]; for (j = 0; j < nregs; j++) df_set_regs_ever_live (hard_regno + j, true); } Index: testsuite/gcc.target/i386/pr55141.c =================================================================== --- testsuite/gcc.target/i386/pr55141.c (revision 0) +++ testsuite/gcc.target/i386/pr55141.c (working copy) @@ -0,0 +1,29 @@ +/* { dg-do run } */ +/* { dg-options "-O -fno-split-wide-types" } */ + +typedef struct +{ + long int p_x, p_y; +} Point; + +static __attribute__ ((noinline, noclone)) + void foo (Point p0, Point p1, Point p2, Point p3) +{ + if (p0.p_x != 1 + || p1.p_x != 3 + || p2.p_x != 5 + || p3.p_x != 7) + __builtin_abort (); +} + +int +main (int argc, char *argv[]) +{ + Point p0, p1, p2, p3, p4, p5; + p0.p_x = 1; + p1.p_x = 3; + p2.p_x = 5; + p3.p_x = 7; + foo (p0, p1, p2, p3); + return 0; +}
The following patch fixes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55141 The patch was successfully bootstrapped on x86/x86-64. Committed as rev. 194308. 2012-12-07 Vladimir Makarov <vmakarov@redhat.com> testsuite/gcc.target/i386/pr55141.c * lra-constraints.c (lra_constraints): Use biggest mode for df_set_regs_ever_live. 2012-12-07 Vladimir Makarov <vmakarov@redhat.com> PR rtl-optimization/55141 * gcc.target/i386/pr55141.c: New.