On Tue, Dec 18, 2012 at 8:03 PM, Jakub Jelinek <ja...@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 <ja...@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
