| Message ID | 20121218190358.GT2315@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 18, 2012 at 8:03 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > On Tue, Dec 18, 2012 at 09:25:14AM +0100, Paolo Bonzini wrote: >> Il 17/12/2012 22:33, Jakub Jelinek ha scritto: >> > If gen_lowpart_if_possible returns NULL, the default >> > rtl_hooks.gen_lowpart_no_emit hook returns the original value, which is not >> > of the desired mode. I bet in most passes for real insns such rtx is then >> > meant to fail recog and thrown away, but for DEBUG_INSN modification that >> > doesn't work, since there is no verification (but also e.g. any kind of >> > SUBREG is fine). So we can e.g. end up with a (plus:SI (mem:DI ...) (mem:SI ...)) >> > or similar and then various passes (in this testcase on s390x reload) can be >> > very upset about that. >> >> Makes sense, and it could even be a wrong-code bug for this simplification: > > Richi reported another related failure today. During combine, > rtl_hooks.gen_lowpart_no_emit is the combine version, which instead of > giving up creates (clobber:MODE (const_int 0)). This is slightly less wrong > than what the general hook did, but still dwarf2out would ICE when seeing > that (with checking, without it just not provide location info). > We can easily emit the SUBREG though (e.g. var-tracking itself also calls > gen_rtx_raw_SUBREG as last resort) in the DEBUG_INSN operands. > > Bootstrapped/regtested on x86_64-linux and i686-linux (and on the testcase > using -> powerpc64-linux cross), ok for trunk? Ok. Thanks, Richard. > 2012-12-18 Jakub Jelinek <jakub@redhat.com> > > PR debug/55730 > * dwarf2out.c (mem_loc_descriptor): Ignore CLOBBER. > * valtrack.c (gen_lowpart_for_debug): New function. > (propagate_for_debug): Temporarily set rtl_hooks.gen_lowpart_no_emit > to gen_lowpart_for_debug. > > * gcc.dg/debug/pr55730.c: New test. > > --- gcc/dwarf2out.c.jj 2012-12-18 11:41:30.000000000 +0100 > +++ gcc/dwarf2out.c 2012-12-18 16:38:26.925380294 +0100 > @@ -12714,6 +12714,7 @@ mem_loc_descriptor (rtx rtl, enum machin > case CONST_VECTOR: > case CONST_FIXED: > case CLRSB: > + case CLOBBER: > /* If delegitimize_address couldn't do anything with the UNSPEC, we > can't express it in the debug info. This can happen e.g. with some > TLS UNSPECs. */ > --- gcc/valtrack.c.jj 2012-11-05 15:02:17.000000000 +0100 > +++ gcc/valtrack.c 2012-12-18 17:15:18.499375776 +0100 > @@ -29,6 +29,24 @@ along with GCC; see the file COPYING3. > #include "regs.h" > #include "emit-rtl.h" > > +/* gen_lowpart_no_emit hook implementation for DEBUG_INSNs. In DEBUG_INSNs, > + all lowpart SUBREGs are valid, despite what the machine requires for > + instructions. */ > + > +static rtx > +gen_lowpart_for_debug (enum machine_mode mode, rtx x) > +{ > + rtx result = gen_lowpart_if_possible (mode, x); > + if (result) > + return result; > + > + if (GET_MODE (x) != VOIDmode) > + return gen_rtx_raw_SUBREG (mode, x, > + subreg_lowpart_offset (mode, GET_MODE (x))); > + > + return NULL_RTX; > +} > + > /* Replace auto-increment addressing modes with explicit operations to access > the same addresses without modifying the corresponding registers. */ > > @@ -158,6 +176,7 @@ propagate_for_debug (rtx insn, rtx last, > basic_block this_basic_block) > { > rtx next, loc, end = NEXT_INSN (BB_END (this_basic_block)); > + rtx (*saved_rtl_hook_no_emit) (enum machine_mode, rtx); > > struct rtx_subst_pair p; > p.to = src; > @@ -165,6 +184,8 @@ propagate_for_debug (rtx insn, rtx last, > > next = NEXT_INSN (insn); > last = NEXT_INSN (last); > + saved_rtl_hook_no_emit = rtl_hooks.gen_lowpart_no_emit; > + rtl_hooks.gen_lowpart_no_emit = gen_lowpart_for_debug; > while (next != last && next != end) > { > insn = next; > @@ -179,6 +200,7 @@ propagate_for_debug (rtx insn, rtx last, > df_insn_rescan (insn); > } > } > + rtl_hooks.gen_lowpart_no_emit = saved_rtl_hook_no_emit; > } > > /* Initialize DEBUG to an empty list, and clear USED, if given. */ > --- gcc/testsuite/gcc.dg/debug/pr55730.c.jj 2012-12-18 17:08:29.649351676 +0100 > +++ gcc/testsuite/gcc.dg/debug/pr55730.c 2012-12-18 17:08:04.000000000 +0100 > @@ -0,0 +1,24 @@ > +/* PR debug/55730 */ > +/* { dg-do compile } */ > +/* { dg-options "-w" } */ > + > +union U > +{ > + float f; > + int i; > +}; > + > +void > +foo (unsigned short *x, unsigned char y) > +{ > + unsigned char g; > + union U u; > + if (u.i < 0) > + g = 0; > + else > + { > + u.f = u.f * (255.0F / 256.0F) + 32768.0F; > + g = (unsigned char) u.i; > + } > + *x = (g << 8) | y; > +} > > Jakub
--- gcc/dwarf2out.c.jj 2012-12-18 11:41:30.000000000 +0100 +++ gcc/dwarf2out.c 2012-12-18 16:38:26.925380294 +0100 @@ -12714,6 +12714,7 @@ mem_loc_descriptor (rtx rtl, enum machin case CONST_VECTOR: case CONST_FIXED: case CLRSB: + case CLOBBER: /* If delegitimize_address couldn't do anything with the UNSPEC, we can't express it in the debug info. This can happen e.g. with some TLS UNSPECs. */ --- gcc/valtrack.c.jj 2012-11-05 15:02:17.000000000 +0100 +++ gcc/valtrack.c 2012-12-18 17:15:18.499375776 +0100 @@ -29,6 +29,24 @@ along with GCC; see the file COPYING3. #include "regs.h" #include "emit-rtl.h" +/* gen_lowpart_no_emit hook implementation for DEBUG_INSNs. In DEBUG_INSNs, + all lowpart SUBREGs are valid, despite what the machine requires for + instructions. */ + +static rtx +gen_lowpart_for_debug (enum machine_mode mode, rtx x) +{ + rtx result = gen_lowpart_if_possible (mode, x); + if (result) + return result; + + if (GET_MODE (x) != VOIDmode) + return gen_rtx_raw_SUBREG (mode, x, + subreg_lowpart_offset (mode, GET_MODE (x))); + + return NULL_RTX; +} + /* Replace auto-increment addressing modes with explicit operations to access the same addresses without modifying the corresponding registers. */ @@ -158,6 +176,7 @@ propagate_for_debug (rtx insn, rtx last, basic_block this_basic_block) { rtx next, loc, end = NEXT_INSN (BB_END (this_basic_block)); + rtx (*saved_rtl_hook_no_emit) (enum machine_mode, rtx); struct rtx_subst_pair p; p.to = src; @@ -165,6 +184,8 @@ propagate_for_debug (rtx insn, rtx last, next = NEXT_INSN (insn); last = NEXT_INSN (last); + saved_rtl_hook_no_emit = rtl_hooks.gen_lowpart_no_emit; + rtl_hooks.gen_lowpart_no_emit = gen_lowpart_for_debug; while (next != last && next != end) { insn = next; @@ -179,6 +200,7 @@ propagate_for_debug (rtx insn, rtx last, df_insn_rescan (insn); } } + rtl_hooks.gen_lowpart_no_emit = saved_rtl_hook_no_emit; } /* Initialize DEBUG to an empty list, and clear USED, if given. */ --- gcc/testsuite/gcc.dg/debug/pr55730.c.jj 2012-12-18 17:08:29.649351676 +0100 +++ gcc/testsuite/gcc.dg/debug/pr55730.c 2012-12-18 17:08:04.000000000 +0100 @@ -0,0 +1,24 @@ +/* PR debug/55730 */ +/* { dg-do compile } */ +/* { dg-options "-w" } */ + +union U +{ + float f; + int i; +}; + +void +foo (unsigned short *x, unsigned char y) +{ + unsigned char g; + union U u; + if (u.i < 0) + g = 0; + else + { + u.f = u.f * (255.0F / 256.0F) + 32768.0F; + g = (unsigned char) u.i; + } + *x = (g << 8) | y; +}