This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add gen_lowpart_for_debug (PR debug/55730)


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]