[Bug debug/106746] New: [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks

[Bug debug/106746] New: [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks

[zsojka at seznam dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

            Bug ID: 106746
           Summary: [13 Regression] '-fcompare-debug' failure (length)
                    with -O2 -fsched2-use-superblocks
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: compare-debug-failure
          Severity: normal
          Priority: P3
         Component: debug
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
                CC: aoliva at gcc dot gnu.org
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu

Created attachment 53511
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53511&action=edit
reduced testcase

Compiler output:
$ x86_64-pc-linux-gnu-gcc -O2 -fsched2-use-superblocks -fcompare-debug
testcase.c -save-temps -Wno-psabi
x86_64-pc-linux-gnu-gcc: error: testcase.c: '-fcompare-debug' failure (length)

$ diff -u *gkd
--- a-testcase.c.gkd    2022-08-26 07:38:56.088210543 +0200
+++ a-testcase.gk.c.gkd 2022-08-26 07:38:56.131543638 +0200
@@ -435,11 +435,6 @@
         ]) "testcase.c":24:5# {*andsi_1_zext}
      (expr_list:REG_UNUSED (reg:CC 17 flags)
         (nil)))
-(insn # 0 0 2 (set (reg:V16QI 23 xmm3 [272])
-        (plus:V16QI (reg:V16QI 23 xmm3 [280])
-            (mem/c:V16QI (plus:DI (reg/f:DI 7 sp)
-                    (const_int -120 [0xffffffffffffff88])) [  S16 A128])))
"testcase.c":25:14# {*addv16qi3}
-     (nil))
 (insn:TI # 0 0 2 (set (reg:V4SI 22 xmm2 [208])
         (vec_concat:V4SI (reg:V2SI 22 xmm2 [224])
             (reg:V2SI 26 xmm6 [221]))) "testcase.c":24:5# {*vec_concatv4si}
@@ -493,12 +488,6 @@
                 (const_int 8 [0x8])) [
VIEW_CONVERT_EXPR<int[16]>(D.xxxx)[_45]+0 S4 A32])) "testcase.c":24:5#
{*movsi_internal}
      (expr_list:REG_DEAD (reg:DI 1 dx [234])
         (nil)))
-(insn # 0 0 2 (set (reg:SI 2 cx [256])
-        (mem:SI (plus:DI (reg/f:DI 7 sp)
-                (const_int 128 [0x80])) [  S4 A64])) "testcase.c":24:5#
{*movsi_internal}
-     (expr_list:REG_EQUIV (mem:SI (plus:DI (reg/f:DI 19 frame)
-                (const_int -200 [0xffffffffffffff38])) [  S4 A64])
-        (nil)))
 (insn:TI # 0 0 2 (set (reg:V2SI 26 xmm6 [241])
         (vec_concat:V2SI (reg:SI 26 xmm6 [241])
             (reg:SI 22 xmm2 [orig:242 VIEW_CONVERT_EXPR<int[16]>(D.xxxx)[_51]
] [242]))) "testcase.c":24:5# {*vec_concatv2si}
@@ -511,6 +500,33 @@
...


Renaming the variable "foo0_v512u32_0" to "b" makes the failure disappear.

$ x86_64-pc-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r13-2206-20220825175413-g60d84e82639-checking-yes-rtl-df-extra-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/13.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu
--with-ld=/usr/bin/x86_64-pc-linux-gnu-ld
--with-as=/usr/bin/x86_64-pc-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r13-2206-20220825175413-g60d84e82639-checking-yes-rtl-df-extra-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.0.0 20220825 (experimental) (GCC)

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lingling.kong7 at gmail dot com,
                   |                            |marxin at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
            Summary|[13 Regression]             |[13 Regression]
                   |'-fcompare-debug' failure   |'-fcompare-debug' failure
                   |(length) with -O2           |(length) with -O2
                   |-fsched2-use-superblocks    |-fsched2-use-superblocks
                   |                            |since
                   |                            |r13-2041-g6624ad73064de241
   Last reconfirmed|                            |2022-08-26

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r13-2041-g6624ad73064de241.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> ---
A slight change in the variable makes the problem to go away.  It looks
like a latent bug.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #3 from H.J. Lu <hjl.tools at gmail dot com> ---
This debug INSN:

(debug_insn 29 28 30 2 (var_location:BLK D#2 (concatn:BLK [
            (mem/j:SI (plus:DI (plus:DI (ashift:DI (zero_extend:DI (and:SI
(mem:SI (plus:DI (reg/f:DI 7 sp) 
                                            (const_int 72 [0x48])) [1  S4
A512])
                                    (const_int 15 [0xf])))
                            (const_int 2 [0x2]))
                        (reg/f:DI 7 sp))
                    (const_int 8 [0x8])) [1
VIEW_CONVERT_EXPR<int[16]>(vectmp.9)[_18]+0 S4 A32])
            (mem/j:SI (plus:DI (plus:DI (ashift:DI (zero_extend:DI (and:SI
(mem:SI (plus:DI (reg/f:DI 7 sp) 
                                            (const_int 76 [0x4c])) [1  S4 A32])
                                    (const_int 15 [0xf])))
                            (const_int 2 [0x2]))
                        (reg/f:DI 7 sp))
                    (const_int 8 [0x8])) [1
VIEW_CONVERT_EXPR<int[16]>(vectmp.9)[_21]+0 S4 A32])
            (mem/j:SI (plus:DI (plus:DI (ashift:DI (zero_extend:DI (and:SI
(mem:SI (plus:DI (reg/f:DI 7 sp) 
                                            (const_int 80 [0x50])) [1  S4 A64])
                                    (const_int 15 [0xf])))

triggered the failure.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> ---
This simple change:

diff --git a/gcc/config/i386/i386-modes.def b/gcc/config/i386/i386-modes.def
index e2e1e18d24d..b49daaef253 100644
--- a/gcc/config/i386/i386-modes.def
+++ b/gcc/config/i386/i386-modes.def
@@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
 FRACTIONAL_FLOAT_MODE (XF, 80, 12, ieee_extended_intel_96_format);
 FLOAT_MODE (TF, 16, ieee_quad_format);
 FLOAT_MODE (HF, 2, ieee_half_format);
+FLOAT_MODE (BF, 2, 0);
+ADJUST_FLOAT_FORMAT (BF, &arm_bfloat_half_format);

 /* In ILP32 mode, XFmode has size 12 and alignment 4.
    In LP64 mode, XFmode has size and alignment 16.  */

triggers this issue.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H.J. Lu from comment #4)
> This simple change:
> 
> diff --git a/gcc/config/i386/i386-modes.def b/gcc/config/i386/i386-modes.def
> index e2e1e18d24d..b49daaef253 100644
> --- a/gcc/config/i386/i386-modes.def
> +++ b/gcc/config/i386/i386-modes.def
> @@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
>  FRACTIONAL_FLOAT_MODE (XF, 80, 12, ieee_extended_intel_96_format);
>  FLOAT_MODE (TF, 16, ieee_quad_format);
>  FLOAT_MODE (HF, 2, ieee_half_format);
> +FLOAT_MODE (BF, 2, 0);
> +ADJUST_FLOAT_FORMAT (BF, &arm_bfloat_half_format);
>  
>  /* In ILP32 mode, XFmode has size 12 and alignment 4.
>     In LP64 mode, XFmode has size and alignment 16.  */
> 
> triggers this issue.

This triggered by r13-1942.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |richard.sandiford at arm dot com,
                   |                            |roger at nextmovesoftware dot com,
                   |                            |segher at kernel dot crashing.org

--- Comment #6 from H.J. Lu <hjl.tools at gmail dot com> ---
Revert

diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index fa20665bb01..7d09bf7103d 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -1615,6 +1639,24 @@ simplify_context::simplify_unary_operation_1 (rtx_code
co
de, machine_mode mode,
            }
        }

+      /* We can canonicalize SIGN_EXTEND (op) as ZERO_EXTEND (op) when
+         we know the sign bit of OP must be clear.  */
+      if (val_signbit_known_clear_p (GET_MODE (op),
+                                    nonzero_bits (op, GET_MODE (op))))
+       return simplify_gen_unary (ZERO_EXTEND, mode, op, GET_MODE (op));
+
+      /* (sign_extend:DI (subreg:SI (ctz:DI ...))) is (ctz:DI ...).  */
+      if (GET_CODE (op) == SUBREG
+         && subreg_lowpart_p (op)
+         && GET_MODE (SUBREG_REG (op)) == mode
+         && is_a <scalar_int_mode> (mode, &int_mode)
+         && is_a <scalar_int_mode> (GET_MODE (op), &op_mode)
+         && GET_MODE_PRECISION (int_mode) <= HOST_BITS_PER_WIDE_INT
+         && GET_MODE_PRECISION (op_mode) < GET_MODE_PRECISION (int_mode)
+         && (nonzero_bits (SUBREG_REG (op), mode)
+             & ~(GET_MODE_MASK (op_mode) >> 1)) == 0)
+       return SUBREG_REG (op);
+
 #if defined(POINTERS_EXTEND_UNSIGNED)
       /* As we do not know which address space the pointer is referring to,
         we can do this only if the target does not support different pointer

fixes the failure.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> ---

(In reply to H.J. Lu from comment #3)
> This debug INSN:
> 
> (debug_insn 29 28 30 2 (var_location:BLK D#2 (concatn:BLK [
>             (mem/j:SI (plus:DI (plus:DI (ashift:DI (zero_extend:DI (and:SI
> (mem:SI (plus:DI (reg/f:DI 7 sp) 
>                                             (const_int 72 [0x48])) [1  S4
> A512])
>                                     (const_int 15 [0xf])))
>                             (const_int 2 [0x2]))
>                         (reg/f:DI 7 sp))
>                     (const_int 8 [0x8])) [1
> VIEW_CONVERT_EXPR<int[16]>(vectmp.9)[_18]+0 S4 A32])
>             (mem/j:SI (plus:DI (plus:DI (ashift:DI (zero_extend:DI (and:SI
> (mem:SI (plus:DI (reg/f:DI 7 sp) 
>                                             (const_int 76 [0x4c])) [1  S4
> A32])
>                                     (const_int 15 [0xf])))
>                             (const_int 2 [0x2]))
>                         (reg/f:DI 7 sp))
>                     (const_int 8 [0x8])) [1
> VIEW_CONVERT_EXPR<int[16]>(vectmp.9)[_21]+0 S4 A32])
>             (mem/j:SI (plus:DI (plus:DI (ashift:DI (zero_extend:DI (and:SI
> (mem:SI (plus:DI (reg/f:DI 7 sp) 
>                                             (const_int 80 [0x50])) [1  S4
> A64])
>                                     (const_int 15 [0xf])))
> 
> triggered the failure.

CONCAT and CONCATN never appear in the insn chain. They may cause different
insn
orders with and without debug insn.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[roger at nextmovesoftware dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #9 from Roger Sayle <roger at nextmovesoftware dot com> ---
I'm curious why the zero_extend behaves so differently to a sign_extend,
perhaps a  missing simplification or pattern.  Presumably the CONCAT in the
debug_insn is there whether or not a sign_extend or zero_extend is used?

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #10 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Roger Sayle from comment #9)
> I'm curious why the zero_extend behaves so differently to a sign_extend,
> perhaps a  missing simplification or pattern.  Presumably the CONCAT in the
> debug_insn is there whether or not a sign_extend or zero_extend is used?

I think instruction order changes at random when there are debug insns
with CONCAT and CONCATN.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |vmakarov at gcc dot gnu.org

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I doubt the trigger is exactly CONCAT/CONCATN, I don't see those mentioned
anywhere in
the scheduler nor in i386 backend code related to scheduling.
But the DEBUG_INSN with CONCATN in this case contains 16 MEMs, perhaps
something defers some insns for later if that DEBUG_INSN is scheduled.
Not very familiar with the scheduling dumps though.
With:
./xgcc -B ./ -S -O2 -fsched2-use-superblocks -fcompare-debug pr106746.c
-Wno-psabi --param min-nondebug-insn-uid=10000 -fdump-tree-all -da
-fsched-verbose=8
I see that it is tick 24 in which insn 10148 is scheduled with -g0 and not
otherwise,
already in tick 21 it is:
 ;;      21-->   10090 xmm7=[dx*0x4+sp+0x8]                   
:hsw_decodern,hsw_p23
+;;             dependencies resolved: insn   10148
+;;             tick updated: insn   10148 into ready
 ;;             dependencies resolved: insn   10203
 ;;             tick updated: insn   10203 into ready
 ;;             dependencies resolved: insn   10202
 ;;             tick updated: insn   10202 into ready
 ;;             dependencies resolved: insn   10201
 ;;             tick updated: insn   10201 into ready
But why, I'm not sure.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Alex, please correct me if I'm wrong, but I think the scheduling DEBUG_INSN
decision was that DEBUG_INSNs are somehow taken into account during scheduling,
not completely ignored, so that they actually move together with scheduling
changes.
I know H.J. has posted a patch to ignore for scheduling purposes everything
inside of CONCAT{,N}, but the scheduler itself doesn't specifically check for
those 2 rtxes anywhere, so I don't find anything special on them.  And as has
been tried, ignoring DEBUG_INSNs for all scheduler decisions regresses a lot
(not unsuprisingly), they should be scheduled after insns which produce/store
what they depend on and before later insns that modify the registers/memroy
used in them if possible (or reset if that would affect
how non-DEBUG_INSNs are scheduled relative to each other).  Though, I think
DEBUG_INSNs should be treated as needing no computing resources, with 0
latencies.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #13 from Segher Boessenkool <segher at gcc dot gnu.org> ---
DEBUG_INSNs should never influence any scheduling decisions, or any other
decisions that influence what machine code we generate.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #13)
> DEBUG_INSNs should never influence any scheduling decisions, or any other
> decisions that influence what machine code we generate.

Well, with the exception of scheduling decisions for the DEBUG_INSNs themselves
(where exactly to place them).
haifa-sched.cc is full of various DEBUG_INSN related checks, in some cases
DEBUG_INSNs are ignored, in other places they are taken into account, ...
And in the end, most of the time it works fine for -fcompare-debug checks; it
doesn't for this testcase, so that needs to be understood why and fixed.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #15 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Yup.  I don't consider DEBUG_INSNs to be scheduled at all, only real things
are, but that is just vague terminology :-)

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #16 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Sorry it took me so long to react, I'd missed the question.

IIRC the scheduler was the hardest part of GCC to make work with debug insns. 
The general strategy is that nondebug insns never depend on debug insns, while
debug insns depend on the preceding insns and on the previous debug insn,
besides the deps it would have if it were a regular insn.

This generally enables debug insns to be issued right after the insn that
produces the side effect it notes, but if the nondebug insn is pulled ahead,
the ordering WRT other debug insns keeps the debug views consistent with the
ordering of side effects in source code.

They shouldn't, however, prevent a nondebug insn from being pulled ahead of
them.  We take note of conflicts to invalidate the expression in the debug
insn, rather than to prevent reordering.

That's the theory, and there have been plenty of deviations from that caught by
-fcompare-debug and fixed during the VTA development, but it was tricky enough
that selective scheduling could never be made VTA-safe.  It's not what's in use
for x86, though, so this must be something else.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #17 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Created attachment 54272
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54272&action=edit
patch that fixes the problem for reasons not fully understood

It seems that looking up the MEM exprs in DEBUG_INSNs disturbs something in
cselib and causes pending MEMs to conflict that, in the non-debug case, don't.

There's no need for these lookups in debug insns, the results aren't used, and
I thought I'd just queue up this improvement but, to my surprise, it made the
problem go away.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Thanks for looking into this.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:3c99493bf39a7fef9213e6f5af94b78bb15fcfdc

commit r13-5252-g3c99493bf39a7fef9213e6f5af94b78bb15fcfdc
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Thu Jan 19 01:09:15 2023 -0300

    [PR106746] drop cselib addr lookup in debug insn mem

    The testcase used to get scheduled differently depending on the
    presence of debug insns with MEMs.  It's not clear to me why those
    MEMs affected scheduling, but the cselib pre-canonicalization of the
    MEM address is not used at all when analyzing debug insns, so the
    memory allocation and lookup are pure waste.  Somehow, avoiding that
    waste fixes the problem, or makes it go latent.


    for  gcc/ChangeLog

            PR debug/106746
            * sched-deps.cc (sched_analyze_2): Skip cselib address lookup
            within debug insns.

    for  gcc/testsuite/ChangeLog

            PR debug/106746
            * gcc.target/i386/pr106746.c: New.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #20 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
The bug is now either fixed or latent in the trunk, I'm not sure which, because
I have not got as far as figuring out why removing unnecessary address cselib
lookups in debug insns made a difference to memory overlap checks between
nondebug insns.  So I'm not sure whether to close this PR or leave it open. 
Thoughts?

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #21 from Segher Boessenkool <segher at gcc dot gnu.org> ---
As far as we (me, you; everybody) can tell it is fixed now.  If one day we get
a testcase showing it has in fact not been fixed, the problem is still there,
we can reopen or link the testcases or etc.?

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jsm28 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #22 from Joseph S. Myers <jsm28 at gcc dot gnu.org> ---
The fix introduced a regression building glibc for ia64-linux-gnu, see bug
108484.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #23 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
See PR108463 and
https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610778.html

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #24 from Segher Boessenkool <segher at gcc dot gnu.org> ---
So this PR can be marked resolved now?

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #25 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Now, I believe the fix was incorrect and the other PR has all the details on
it.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #25)
> Now, I believe the fix was incorrect and the other PR has all the details on
> it.

S/Now/No/, ouch.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

--- Comment #27 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:465a9c51e7d5bafa7a81195b5af20f2a54f22210

commit r13-5652-g465a9c51e7d5bafa7a81195b5af20f2a54f22210
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Feb 2 13:52:45 2023 +0100

    sched-deps, cselib: Fix up some -fcompare-debug issues and regressions
[PR108463]

    On Sat, Jan 14, 2023 at 08:26:00AM -0300, Alexandre Oliva via Gcc-patches
wrote:
    > The testcase used to get scheduled differently depending on the
    > presence of debug insns with MEMs.  It's not clear to me why those
    > MEMs affected scheduling, but the cselib pre-canonicalization of the
    > MEM address is not used at all when analyzing debug insns, so the
    > memory allocation and lookup are pure waste.  Somehow, avoiding that
    > waste fixes the problem, or makes it go latent.

    Unfortunately, this patch breaks the following testcase.
    The code in sched_analyze_2 did 2 things:
    1) cselib_lookup_from_insn
    2) shallow_copy_rtx + cselib_subst_to_values_from_insn
    Now, 1) is precondition of 2), we can only subst the VALUEs if we
    have actually looked the address up, but as can be seen on that testcase,
    we are relying on at least the 1) to be done because we subst the values
    later on even on DEBUG_INSNs and actually use those when needed.
    cselib_subst_to_values_from_insn mostly just replaces stuff in the
    returned rtx, except for:
          /* This used to happen for autoincrements, but we deal with them
             properly now.  Remove the if stmt for the next release.  */
          if (! e)
            {
              /* Assign a value that doesn't match any other.  */
              e = new_cselib_val (next_uid, GET_MODE (x), x);
            }
    which is like that since 2011, I hope it is never reachable and we should
    in stage1 replace that with gcc_assert or just remove (then it will
    segfault on following
          return e->val_rtx;
    ).

    So, I (as done in the patch below) reinstalled the 1) and not 2) for
    DEBUG_INSNs.  This fixed the new testcase, but broke again the PR106746
    testcases.

    I've spent a day debugging that and found the problem is that as documented
    in a large comment in cselib.cc above n_useless_values variable definition,
    we spend quite a few effort on making sure that VALUEs created on
    DEBUG_INSNs don't affect the cselib decisions for non-DEBUG_INSNs such as
    pruning of useless values etc., but if a VALUE created that way is then
    looked up/needed from non-DEBUG_INSNs, we promote it to non-debug.

    The reason for -fcompare-debug failure is that there is one large
DEBUG_INSN
    with 16 MEMs in it mostly with addresses that so far didn't appear in the
IL
    otherwise.  Later on, we see an instruction storing into MEM destination
    and invalidate that MEM.  Unfortunately, there is a bug caused by the
    introduction of SP_DERIVED_VALUE_P where alias.cc isn't able to
disambiguate
    MEMs with sp + optional offset in address vs. MEMs with address being a
    VALUE having SP_DERIVED_VALUE_P + constant (or the SP_DERIVED_VALUE_P
    itself), which ought to be possible when REG_VALUES (REGNO
    (stack_pointer_rtx)) has SP_DERIVED_VALUE_P + constant location.  Not sure
    if I should try to fix that in stage4 or defer for stage1.
    Anyway, the cselib_invalidate_mem call because of this invalidates
basically
    all MEMs with the exception of 5 which have MEM_EXPRs that guarantee
    non-aliasing with the sp based store.
    Unfortunately, n_useless_values which in my understanding should be always
    the same between -g and -g0 compilations diverges, has 3 more useless
values
    for -g.

    Now, these were initially VALUEs created for DEBUG_INSN lookups.  As I
said,
    cselib.cc has code to promote such VALUEs (well, their location elements)
to
    non-debug if they are looked up from non-DEBUG_INSNs.  The problem is that
    when looking some completely unrelated MEM from a non-DEBUG_INSN we run
into
    a hash collision and so call cselib_hasher::equal to check if the unrelated
    MEM is equal to the one from DEBUG_INSN only element.  The equal static
    member function calls rtx_equal_for_cselib_1 and if that returns true,
    promotes the location to non-DEBUG, otherwise returns false.  So far so
    good.  But rtx_equal_for_cselib_1 internally performs various other cselib
    lookups, all done with the non-DEBUG_INSN cselib_current_insn, so they
    all promote to non-debug.  And that is wrong, because if it was -g0
    compilation, such hashtable entry wouldn't be there at all (or would be
    but wouldn't contain that locs element), so with -g0 we wouldn't call
    that rtx_equal_for_cselib_1 at all.  So, I think we need to pretend
    that such lookup which only happens with -g and not -g0 actually comes
    from some DEBUG_INSN (note, the lookups rtx_equal_for_cselib_1 does
    are always with create = 0).
    The cselib.cc part of the patch does that.

    BTW, I'm not really sure how:
              if (num_mems < param_max_cselib_memory_locations
                  && ! canon_anti_dependence (x, false, mem_rtx,
                                              GET_MODE (mem_rtx), mem_addr))
                {
                  has_mem = true;
                  num_mems++;
                  p = &(*p)->next;
                  continue;
                }
    num_mems cap can actually work correctly for -fcompare-debug,
    I'd think we would need to differentiate between num_debug_mems and
    num_mems depending on if setting_insn is non-NULL DEBUG_INSN or not.
    That was one of my suspicions on this testcase, but the number of MEMs
    was small enough for the param in either case (especially because of
    the above mentioned missed non-aliasings).  But as implemented, I think
    if we have tons of non-aliased MEMs from DEBUG_INSN setting_insns,
    we could unchain lots more non-DEBUG MEMs with -g than with -g0.

    2023-02-02  Jakub Jelinek  <jakub@redhat.com>

            PR debug/106746
            PR rtl-optimization/108463
            PR target/108484
            * cselib.cc (cselib_current_insn): Move declaration earlier.
            (cselib_hasher::equal): For debug only locs, temporarily override
            cselib_current_insn to their l->setting_insn for the
            rtx_equal_for_cselib_1 call, so that unsuccessful comparisons don't
            promote some debug locs.
            * sched-deps.cc (sched_analyze_2) <case MEM>: For MEMs in
DEBUG_INSNs
            when using cselib call cselib_lookup_from_insn on the address but
            don't substitute it.

            * gcc.dg/pr108463.c: New test.

[Bug debug/106746] [13 Regression] '-fcompare-debug' failure (length) with -O2 -fsched2-use-superblocks since r13-2041-g6624ad73064de241

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106746

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #28 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed now.