Message ID | 20150407142311.GH19273@tucnak.redhat.com |
---|---|
State | New |
Headers | show |
On Tue, 7 Apr 2015, Jakub Jelinek wrote: > Hi! > > gen_rtx_SUBREG/gen_lowpart_SUBREG ICE when trying to create a subreg > the backend doesn't like, lowpart_subreg returns NULL. But, for debug info > purposes, all lowpart subregs are valid, a subreg in the debug insns or > var-tracking notes is simply a mode punning construct, and even when the > backend doesn't have any instruction to perform such operation, the debugger > can always do the punning. > > var-tracking.c already has code to emit gen_rtx_raw_SUBREG if everything > else fails, the following patch adjusts valtrack.c to do the same thing. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. > 2015-04-07 Jakub Jelinek <jakub@redhat.com> > > PR debug/65678 > * valtrack.c (debug_lowpart_subreg): New function. > (dead_debug_insert_temp): Use it. > > * g++.dg/debug/pr65678.C: New test. > > --- gcc/valtrack.c.jj 2015-01-05 13:07:14.000000000 +0100 > +++ gcc/valtrack.c 2015-04-07 11:10:36.045912355 +0200 > @@ -534,6 +534,22 @@ dead_debug_add (struct dead_debug_local > bitmap_set_bit (debug->used, uregno); > } > > +/* Like lowpart_subreg, but if a subreg is not valid for machine, force > + it anyway - for use in debug insns. */ > + > +static rtx > +debug_lowpart_subreg (machine_mode outer_mode, rtx expr, > + machine_mode inner_mode) > +{ > + if (inner_mode == VOIDmode) > + inner_mode = GET_MODE (expr); > + int offset = subreg_lowpart_offset (outer_mode, inner_mode); > + rtx ret = simplify_gen_subreg (outer_mode, expr, inner_mode, offset); > + if (ret) > + return ret; > + return gen_rtx_raw_SUBREG (outer_mode, expr, offset); > +} > + > /* If UREGNO is referenced by any entry in DEBUG, emit a debug insn > before or after INSN (depending on WHERE), that binds a (possibly > global) debug temp to the widest-mode use of UREGNO, if WHERE is > @@ -662,9 +678,9 @@ dead_debug_insert_temp (struct dead_debu > /* Ok, it's the same (hardware) REG, but with a different > mode, so SUBREG it. */ > else > - breg = lowpart_subreg (GET_MODE (reg), > - cleanup_auto_inc_dec (src, VOIDmode), > - GET_MODE (dest)); > + breg = debug_lowpart_subreg (GET_MODE (reg), > + cleanup_auto_inc_dec (src, VOIDmode), > + GET_MODE (dest)); > } > else if (GET_CODE (dest) == SUBREG) > { > @@ -684,9 +700,9 @@ dead_debug_insert_temp (struct dead_debu > breg = NULL; > /* Yay, we can use SRC, just adjust its mode. */ > else > - breg = lowpart_subreg (GET_MODE (reg), > - cleanup_auto_inc_dec (src, VOIDmode), > - GET_MODE (dest)); > + breg = debug_lowpart_subreg (GET_MODE (reg), > + cleanup_auto_inc_dec (src, VOIDmode), > + GET_MODE (dest)); > } > /* Oh well, we're out of luck. */ > else > @@ -740,7 +756,8 @@ dead_debug_insert_temp (struct dead_debu > *DF_REF_REAL_LOC (cur->use) = dval; > else > *DF_REF_REAL_LOC (cur->use) > - = gen_lowpart_SUBREG (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval); > + = debug_lowpart_subreg (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval, > + GET_MODE (dval)); > /* ??? Should we simplify subreg of subreg? */ > bitmap_set_bit (debug->to_rescan, INSN_UID (DF_REF_INSN (cur->use))); > uses = cur->next; > --- gcc/testsuite/g++.dg/debug/pr65678.C.jj 2015-04-07 11:18:51.701891845 +0200 > +++ gcc/testsuite/g++.dg/debug/pr65678.C 2015-04-07 11:19:50.285943861 +0200 > @@ -0,0 +1,35 @@ > +// PR debug/65678 > +// { dg-do compile } > + > +long long v; > + > +static int > +bar (double x) > +{ > +#if __SIZEOF_DOUBLE__ == __SIZEOF_LONG_LONG__ > + __builtin_memmove (&v, &x, sizeof v); > +#else > + (void) x; > + v = 0; > +#endif > + return v; > +} > + > +struct A > +{ > + A (double x) : a (bar (x)) {} > + int m1 (); > + int m2 () { int b = a; return b; } > + int a; > +}; > + > +void foo (); > + > +void > +baz (double x) > +{ > + int c = A (x).m2 (); > + int d = A (x).m1 (); > + if (d) > + foo (); > +} > > Jakub > >
Jakub Jelinek <jakub@redhat.com> writes: > Hi! > > gen_rtx_SUBREG/gen_lowpart_SUBREG ICE when trying to create a subreg > the backend doesn't like, lowpart_subreg returns NULL. But, for debug info > purposes, all lowpart subregs are valid, a subreg in the debug insns or > var-tracking notes is simply a mode punning construct, and even when the > backend doesn't have any instruction to perform such operation, the debugger > can always do the punning. > > var-tracking.c already has code to emit gen_rtx_raw_SUBREG if everything > else fails, the following patch adjusts valtrack.c to do the same thing. Looks like this has the potential to introduce nested subregs. Also, the problem isn't just that the architecture can't do some accesses easily (such as accessing the high part of a register). It's also that GCC's model of subregs doesn't always give the right answer. E.g. sometimes the endianess of a particular register is the opposite of what GCC expects, or the value is modelled as occupying more than one register but isn't evenly distributed through them. I take your point about this being done in var-tracking.c, but it always seemed dangerous there too... Thanks, Richard
--- gcc/valtrack.c.jj 2015-01-05 13:07:14.000000000 +0100 +++ gcc/valtrack.c 2015-04-07 11:10:36.045912355 +0200 @@ -534,6 +534,22 @@ dead_debug_add (struct dead_debug_local bitmap_set_bit (debug->used, uregno); } +/* Like lowpart_subreg, but if a subreg is not valid for machine, force + it anyway - for use in debug insns. */ + +static rtx +debug_lowpart_subreg (machine_mode outer_mode, rtx expr, + machine_mode inner_mode) +{ + if (inner_mode == VOIDmode) + inner_mode = GET_MODE (expr); + int offset = subreg_lowpart_offset (outer_mode, inner_mode); + rtx ret = simplify_gen_subreg (outer_mode, expr, inner_mode, offset); + if (ret) + return ret; + return gen_rtx_raw_SUBREG (outer_mode, expr, offset); +} + /* If UREGNO is referenced by any entry in DEBUG, emit a debug insn before or after INSN (depending on WHERE), that binds a (possibly global) debug temp to the widest-mode use of UREGNO, if WHERE is @@ -662,9 +678,9 @@ dead_debug_insert_temp (struct dead_debu /* Ok, it's the same (hardware) REG, but with a different mode, so SUBREG it. */ else - breg = lowpart_subreg (GET_MODE (reg), - cleanup_auto_inc_dec (src, VOIDmode), - GET_MODE (dest)); + breg = debug_lowpart_subreg (GET_MODE (reg), + cleanup_auto_inc_dec (src, VOIDmode), + GET_MODE (dest)); } else if (GET_CODE (dest) == SUBREG) { @@ -684,9 +700,9 @@ dead_debug_insert_temp (struct dead_debu breg = NULL; /* Yay, we can use SRC, just adjust its mode. */ else - breg = lowpart_subreg (GET_MODE (reg), - cleanup_auto_inc_dec (src, VOIDmode), - GET_MODE (dest)); + breg = debug_lowpart_subreg (GET_MODE (reg), + cleanup_auto_inc_dec (src, VOIDmode), + GET_MODE (dest)); } /* Oh well, we're out of luck. */ else @@ -740,7 +756,8 @@ dead_debug_insert_temp (struct dead_debu *DF_REF_REAL_LOC (cur->use) = dval; else *DF_REF_REAL_LOC (cur->use) - = gen_lowpart_SUBREG (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval); + = debug_lowpart_subreg (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval, + GET_MODE (dval)); /* ??? Should we simplify subreg of subreg? */ bitmap_set_bit (debug->to_rescan, INSN_UID (DF_REF_INSN (cur->use))); uses = cur->next; --- gcc/testsuite/g++.dg/debug/pr65678.C.jj 2015-04-07 11:18:51.701891845 +0200 +++ gcc/testsuite/g++.dg/debug/pr65678.C 2015-04-07 11:19:50.285943861 +0200 @@ -0,0 +1,35 @@ +// PR debug/65678 +// { dg-do compile } + +long long v; + +static int +bar (double x) +{ +#if __SIZEOF_DOUBLE__ == __SIZEOF_LONG_LONG__ + __builtin_memmove (&v, &x, sizeof v); +#else + (void) x; + v = 0; +#endif + return v; +} + +struct A +{ + A (double x) : a (bar (x)) {} + int m1 (); + int m2 () { int b = a; return b; } + int a; +}; + +void foo (); + +void +baz (double x) +{ + int c = A (x).m2 (); + int d = A (x).m1 (); + if (d) + foo (); +}