On Tue, May 2, 2023 at 4:08 PM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi!
>
> The following testcase ICEs because STV replaces there
> (debug_insn 114 47 51 8 (var_location:TI D#3 (reg:TI 91 [ p ])) -1
> (nil))
> with
> (debug_insn 114 47 51 8 (var_location:TI D#3 (reg:V1TI 91 [ p ])) -1
> (nil))
> which is invalid because of the mode mismatch.
> STV has fix_debug_reg_uses function which is supposed to fix this up
> and adjust such debug insns into
> (debug_insn 114 47 51 8 (var_location:TI D#3 (subreg:TI (reg:V1TI 91 [ p ])
> 0)) -1
> (nil))
> but it doesn't trigger here.
> The IL before stv1 has:
> (debug_insn 114 47 51 8 (var_location:TI D#3 (reg:TI 91 [ p ])) -1
> (nil))
> ...
> (insn 63 62 64 8 (set (mem/c:TI (reg/f:DI 89 [ .result_ptr ]) [0
> <retval>.mStorage+0 S16 A32])
> (reg:TI 91 [ p ])) "pr109676.C":4:48 87 {*movti_internal}
> (expr_list:REG_DEAD (reg:TI 91 [ p ])
> (nil)))
> in bb 8 and
> (insn 97 96 98 9 (set (reg:TI 91 [ p ])
> (mem/c:TI (plus:DI (reg/f:DI 19 frame)
> (const_int -32 [0xffffffffffffffe0])) [0 p+0 S16 A128]))
> "pr109676.C":26:12 87 {*movti_internal}
> (nil))
> (insn 98 97 99 9 (set (mem/c:TI (plus:DI (reg/f:DI 19 frame)
> (const_int -64 [0xffffffffffffffc0])) [0 tmp+0 S16 A128])
> (reg:TI 91 [ p ])) "pr109676.C":26:12 87 {*movti_internal}
> (nil))
> in bb9.
> PUT_MODE on a REG is done in two spots in timode_scalar_chain::convert_insn,
> one is:
> switch (GET_CODE (dst))
> {
> case REG:
> if (GET_MODE (dst) == TImode)
> {
> PUT_MODE (dst, V1TImode);
> fix_debug_reg_uses (dst);
> }
> if (GET_MODE (dst) == V1TImode)
> when seeing the REG in SET_DEST and another one the hunk the patch adjusts.
> Because bb 8 comes first in the order the pass walks the bbs, we first
> notice the TImode pseudo on insn 63 where it is SET_SRC, use PUT_MODE there
> unconditionally, so for a shared REG it changes all other uses in the IL,
> and then don't call fix_debug_reg_uses because DF_REG_DEF_CHAIN (REGNO (src))
> is non-NULL - the REG is set in insn 97 but we haven't processed it yet.
> Later on we process insn 97, but because the REG in SET_DEST already has
> V1TImode, we don't do anything, even when the src handling code earlier
> relied on it being done.
>
> The following patch fixes this by using similar code for both dst and src,
> in particular calling fix_debug_reg_uses once when we actually change REG
> mode from TImode to V1TImode, and not later on.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/13.2?
Thanks for the clear explanation, the patch LGTM.
>
> 2023-05-02 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/109676
> * config/i386/i386-features.cc (timode_scalar_chain::convert_insn):
> If src is REG, change its mode to V1TImode and call fix_debug_reg_uses
> for it only if it still has TImode. Don't decide whether to call
> fix_debug_reg_uses based on whether SRC is ever set or not.
>
> * g++.target/i386/pr109676.C: New test.
>
> --- gcc/config/i386/i386-features.cc.jj 2023-03-03 11:18:33.100296735 +0100
> +++ gcc/config/i386/i386-features.cc 2023-05-01 14:50:45.559668773 +0200
> @@ -1635,10 +1635,11 @@ timode_scalar_chain::convert_insn (rtx_i
> switch (GET_CODE (src))
> {
> case REG:
> - PUT_MODE (src, V1TImode);
> - /* Call fix_debug_reg_uses only if SRC is never defined. */
> - if (!DF_REG_DEF_CHAIN (REGNO (src)))
> - fix_debug_reg_uses (src);
> + if (GET_MODE (src) == TImode)
> + {
> + PUT_MODE (src, V1TImode);
> + fix_debug_reg_uses (src);
> + }
> break;
>
> case MEM:
> --- gcc/testsuite/g++.target/i386/pr109676.C.jj 2023-05-01 14:58:09.024329438
> +0200
> +++ gcc/testsuite/g++.target/i386/pr109676.C 2023-05-01 14:57:46.566650471
> +0200
> @@ -0,0 +1,46 @@
> +// PR debug/109676
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -g -march=alderlake" }
> +
> +template <class T>
> +struct A {
> + T a;
> + char b;
> + template <typename U>
> + A (U x, int) : a{x} {}
> + A (...);
> + T foo () { return a; }
> +};
> +bool bar ();
> +struct B { int c, d; unsigned char e[8]; };
> +bool baz ();
> +struct C { C () : f () {} B &boo () { return f; } B f; };
> +
> +A<B>
> +qux ()
> +{
> + {
> + A<B> p;
> + bool t = true;
> + for (; bar ();)
> + if (baz ())
> + {
> + t = false;
> + break;
> + }
> + if (t)
> + p.b = false;
> + return p;
> + }
> +}
> +
> +A<C>
> +garply ()
> +{
> + C g;
> + A<B> h = qux ();
> + if (!h.b)
> + return 0;
> + g.boo () = h.foo ();
> + return A<C>{g, 0};
> +}
>
> Jakub
>
--
BR,
Hongtao
