[Bug rtl-optimization/105091] New: RTL dse1 remove stack mem storing incorrectly

[Bug rtl-optimization/105091] New: RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

            Bug ID: 105091
           Summary: RTL dse1 remove stack mem storing incorrectly
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: guojiufu at gcc dot gnu.org
  Target Milestone: ---

Code is narrowed from math/big which passes at the early of r12(r12-656). 
With -fdisable-rtl-dse1, the case also passes. 

----_testmain.go
package main

import "fmt"
import "testing"
import "testing/internal/testdeps"
import __os__ "os"

type Bits []int

func TestMulBits(t *testing.T) {
        for _, test := range []struct {
                x, y, want Bits
        }{

                {Bits{}, Bits{}, nil},
                {Bits{0}, Bits{0}, Bits{0}},
        } {
                p := test.x
                fmt.Printf("%v", p)
        }
}

var tests = []testing.InternalTest {
        {"TestMulBits", TestMulBits},
}
var benchmarks = []testing.InternalBenchmark{
}
var fuzzTargets = []testing.InternalFuzzTarget{
}
var examples = []testing.InternalExample{
}

func main() {
        m := testing.MainStart(testdeps.TestDeps{}, tests, benchmarks,
fuzzTargets, examples)

        __os__.Exit(m.Run())
}
-----

> gccgo -O2 _testmain.go && ./a.out -test.v
=== RUN   TestMulBits
[35185086168544 32199672319005300 268454424 268599296 35185086167968 15 32
35185086167968 15 35184405205936 824635867296 15 35184393891632 0
35185086168112]--- FAIL: TestMulBits (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference
[recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x200000d08280]

goroutine 17 [running]:
testing.tRunner..func3
        /home/guojiufu/gcc/gcc-mainline-base/libgo/go/testing/testing.go:1425
testing.tRunner..func1
        /home/guojiufu/gcc/gcc-mainline-base/libgo/go/testing/testing.go:1342
panic
        /home/guojiufu/gcc/gcc-mainline-base/libgo/go/runtime/panic.go:714
reflect.typedmemmove
        /home/guojiufu/gcc/gcc-mainline-base/libgo/go/runtime/mbarrier.go:197
reflect.packEface
        /home/guojiufu/gcc/gcc-mainline-base/libgo/go/reflect/value.go:123
reflect.valueInterface
        /home/guojiufu/gcc/gcc-mainline-base/libgo/go/reflect/value.go:930
reflect.Value.Interface
        /home/guojiufu/gcc/gcc-mainline-base/libgo/go/reflect/value.go:890
fmt.pp.printValue
        /home/guojiufu/gcc/gcc-mainline-base/libgo/go/fmt/print.go:722
....


> gccgo -O2 _testmain.go -fdisable-rtl-dse1  && ./a.out -test.v
go1: note: disable pass rtl-dse1 for functions in the range of [0, 4294967295]
=== RUN   TestMulBits
[][0]--- PASS: TestMulBits (0.00s)
PASS

> gccgo -v
Using built-in specs.
COLLECT_GCC=/home/guojiufu/gcc/install/gcc-mainline-base-debug/bin/gccgo
COLLECT_LTO_WRAPPER=/home/guojiufu/gcc/install/gcc-mainline-base-debug/libexec/gcc/powerpc64le-unknown-linux-gnu/12.0.1/lto-wrapper
Target: powerpc64le-unknown-linux-gnu
Configured with: /home/guojiufu/gcc/gcc-mainline-base/configure
--enable-languages=c,c++,go --with-cpu=native --enable-checking
--with-long-double-128
--prefix=/home/guojiufu/gcc/install/gcc-mainline-base-debug --disable-bootstrap
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.0.1 20220318 (experimental) (GCC)


I encounter this on ppc64le and did not reproduce it x86_64.

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #1 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
Checking the dumps from dse1, some "stack memory store" are deleted
incorrectly.

   12: %3:DI=call [`runtime.newobject'] argc:0
      REG_CALL_DECL `runtime.newobject'
   13: r117:DI=%3:DI
      REG_DEAD %3:DI
   14: [sfp:DI+0x20]=r117:DI
      REG_DEAD r117:DI
dse1 removes instruction 14.

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #2 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
starting to process insn 14
  v:  1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40,
41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60,
61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80,
81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99,
100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115,
116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131,
132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147,
148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163,
164, 165, 166, 167, 168, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203,
204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216
i = 32, index = 1
i = 33, index = 2
i = 34, index = 3
i = 35, index = 4
i = 36, index = 5
i = 37, index = 6
i = 38, index = 7
i = 39, index = 8
deferring deletion of insn with uid = 14.

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Target|                            |powerpc64le-unknown-linux-g
                   |                            |nu
                 CC|                            |iant at google dot com,
                   |                            |segher at gcc dot gnu.org

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Can you attach the full RTL dump before dse1 and the dse1 dump (don't use -slim
please).

Maybe it's possible to reduce a go testcase to something that can be consumed
by go1 of a cross compiler w/o building target libgo (a "cc1-cross").

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #4 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
Created attachment 52708
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52708&action=edit
279r.cse2

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #5 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
Created attachment 52709
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52709&action=edit
280r.dse1

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #6 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
---bits_test.go
package big

import (
        "fmt"
        "testing"
)

type Bits []int

func TestMulBits(t *testing.T) {
        for _, test := range []struct {
                x, y, want Bits
        }{

                {Bits{}, Bits{}, nil},
                {Bits{0}, Bits{0}, Bits{0}},
        } {
                p := test.x
                fmt.Printf("%v", p)
        }
}

---

Hi Richard!

The dumps are attached.  Thanks.
One interesting thing: after r12-656, it seems no changes on dse.cc relates to
this issue.

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #7 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
tried to remove 'fmt' from the narrowed code, but it is still in code :)

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[ian at airs dot com]

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

Ian Lance Taylor <ian at airs dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ian at airs dot com

--- Comment #8 from Ian Lance Taylor <ian at airs dot com> ---
This program should print

0
1

but when I run it on gcc112.fsffrance.org, compiling with -O2, it prints

1
824633846216


package main

func main() {
        for _, test := range []struct {
                x, y, want []int
        }{
                {[]int{}, []int{}, nil},
                {[]int{0}, []int{0}, []int{0}},
        } {
                p := test.x
                F(p)
        }
}

func F(v interface{}) {
     recover()
     println(cap(v.([]int)))
}

This can be compiled (though not run) using a cross-compiler without building
libgo.

The code coming into 280r.dse1 seems to be indexing from the end of the array. 
I see

code_label 96 126 55 4 118 (nil) [0 uses])
(note 55 96 56 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
(insn 56 55 57 4 (set (reg:DI 144)
        (mult:DI (reg:DI 121 [ ivtmp_47 ])
            (const_int -72 [0xffffffffffffffb8]))) "foo.go":4:2 154 {muldi3}
     (nil))
(insn 57 56 59 4 (set (reg/f:DI 145)
        (plus:DI (reg/f:DI 173)
            (reg:DI 144))) "foo.go":4:2 66 {*adddi3}
     (expr_list:REG_DEAD (reg/f:DI 173)
        (expr_list:REG_DEAD (reg:DI 144)
            (nil))))

where earlier I see

(insn 17 16 19 2 (set (mem/f/c:DI (plus:DI (reg/f:DI 110 sfp)
                (const_int 32 [0x20])) [8 GOTMP.5[0].x.__values+0 S8 A128])
        (reg/f:DI 117 [ _11 ])) "foo.go":4:23 670 {*movdi_internal64}
     (expr_list:REG_DEAD (reg/f:DI 117 [ _11 ])
        (nil)))

and

(insn 120 4 121 2 (set (reg/f:DI 173)
        (plus:DI (reg/f:DI 110 sfp)
            (const_int 32 [0x20]))) 66 {*adddi3}
     (nil))

So register 173 is &GOTMP.5 although insn 120 doesn't indicate that.  Then the
280r.dse1 pass drops out all the assignments to GOTMP.5, presumably because it
doesn't understand that register 173 points to it.

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #9 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
(In reply to Ian Lance Taylor from comment #8)
...
> 
> package main
> 
> func main() {
> 	for _, test := range []struct {
> 		x, y, want []int
> 	}{
> 		{[]int{}, []int{}, nil},
> 		{[]int{0}, []int{0}, []int{0}},
> 	} {
> 		p := test.x
> 		F(p)
> 	}
> }
> 
> func F(v interface{}) {
>      recover()
>      println(cap(v.([]int)))
> }
> 
> This can be compiled (though not run) using a cross-compiler without
> building libgo.
> 
> The code coming into 280r.dse1 seems to be indexing from the end of the
> array.  I see
> 
> code_label 96 126 55 4 118 (nil) [0 uses])
> (note 55 96 56 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
> (insn 56 55 57 4 (set (reg:DI 144)
>         (mult:DI (reg:DI 121 [ ivtmp_47 ])
>             (const_int -72 [0xffffffffffffffb8]))) "foo.go":4:2 154 {muldi3}
>      (nil))
> (insn 57 56 59 4 (set (reg/f:DI 145)
>         (plus:DI (reg/f:DI 173)
>             (reg:DI 144))) "foo.go":4:2 66 {*adddi3}
>      (expr_list:REG_DEAD (reg/f:DI 173)
>         (expr_list:REG_DEAD (reg:DI 144)
>             (nil))))
> 
> where earlier I see
> 
> (insn 17 16 19 2 (set (mem/f/c:DI (plus:DI (reg/f:DI 110 sfp)
>                 (const_int 32 [0x20])) [8 GOTMP.5[0].x.__values+0 S8 A128])
>         (reg/f:DI 117 [ _11 ])) "foo.go":4:23 670 {*movdi_internal64}
>      (expr_list:REG_DEAD (reg/f:DI 117 [ _11 ])
>         (nil)))
> 
> and
> 
> (insn 120 4 121 2 (set (reg/f:DI 173)
>         (plus:DI (reg/f:DI 110 sfp)
>             (const_int 32 [0x20]))) 66 {*adddi3}
>      (nil))
> 
> So register 173 is &GOTMP.5 although insn 120 doesn't indicate that.  Then
> the 280r.dse1 pass drops out all the assignments to GOTMP.5, presumably
> because it doesn't understand that register 173 points to it.

Hi Ian!

Thanks for your great help!

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #10 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
Created attachment 52718
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52718&action=edit
m.go sub1.go

Based on Ian's code, the below code also reproduce this issue.
package sub1

func TestBits(callback func(interface{})) {
        for _, test := range []struct {
                x, y, want []int
        }{
                {[]int{}, nil, nil},
                {[]int{0}, nil, nil},
        } {
                p := test.x
                callback(p)
        }
}

---
> go1 sub1.go -quiet -O2 -o sub1.s

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #11 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
Find one difference between trunk and r12-656:

On trunk:
tree expr = MEM_EXPR (mem); 
   where mem is 
(mem/f/c:DI (plus:DI (reg/f:DI 110 sfp)
         (const_int 32 [0x20])) [3 GOTMP.2[0].x.__values+0 S8 A128])
and then expr is GOTMP.2[0].x.__values

base = get_base_address (expr);   base is "GOTMP.2"
"may_be_aliased (base)" returns false. 

On r12-656: "may_be_aliased (base)" returns true.

may_be_aliased checks TREE_ADDRESSABLE which also returns differences between
trunk and r12-656.

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #12 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
In dse.cc, "may_be_aliased" affects "can_escape" and then affects
"kill_on_calls".

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-03-30

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
  <bb 3> [local count: 715863673]:
  # ivtmp_47 = PHI <1(3), 2(2)>
  _43 = ivtmp_47 * 18446744073709551544;
  GOTMP.8 = MEM[(struct  *)&GOTMP.5 + 144B + _43 * 1];
  GOTMP.13 = GOTMP.8;
  test = GOTMP.13;
  p = test.x;
  D.483.__type_descriptor = &g.type.._6_7int;
  GOTMP.14 = p;
  D.483.__object = &GOTMP.14;
  main.F (D.483);
  p ={v} {CLOBBER(eol)};
  if (ivtmp_47 != 1)
    goto <bb 3>; [66.67%]
  else
    goto <bb 4>; [33.33%]

  <bb 4> [local count: 357878152]:
  test ={v} {CLOBBER(eol)};
  return;

that's an interesting sequence of aggregate copies (meh - SRA should
optimize those!) and an interesting choice of IVs but nothing invalid.

We expand the

  GOTMP.8 = MEM[(struct  *)&GOTMP.5 + 144B + _43 * 1];

stmt to memcpy() but GOTMP.5 is not TREE_ADDRESSABLE here.  That might
in the end lead DSE to remove the stores.  When setting the flag
inside gdb during expansion I see the stores are retained.

Now, emit_block_op_via_libcall does

  /* Since dst and src are passed to a libcall, mark the corresponding
     tree EXPR as addressable.  */
  tree dst_expr = MEM_EXPR (dst);
  tree src_expr = MEM_EXPR (src);
  if (dst_expr)
    mark_addressable (dst_expr);
  if (src_expr)
    mark_addressable (src_expr);

but mark_addressable doesn't handle TARGET_MEM_REF.

Does the following fix the runtime error?  The RTL after DSE seems to be OK.

diff --git a/gcc/gimple-expr.cc b/gcc/gimple-expr.cc
index f9a650b5daf..5faaf43eaf5 100644
--- a/gcc/gimple-expr.cc
+++ b/gcc/gimple-expr.cc
@@ -910,7 +910,8 @@ mark_addressable (tree x)
     x = TREE_OPERAND (x, 0);
   while (handled_component_p (x))
     x = TREE_OPERAND (x, 0);
-  if (TREE_CODE (x) == MEM_REF
+  if ((TREE_CODE (x) == MEM_REF
+       || TREE_CODE (x) == TARGET_MEM_REF)
       && TREE_CODE (TREE_OPERAND (x, 0)) == ADDR_EXPR)
     x = TREE_OPERAND (TREE_OPERAND (x, 0), 0);
   if (!VAR_P (x)

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[guojiufu at gcc dot gnu.org]

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

--- Comment #14 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #13)
...
> Does the following fix the runtime error?  The RTL after DSE seems to be OK.
> 
> diff --git a/gcc/gimple-expr.cc b/gcc/gimple-expr.cc
> index f9a650b5daf..5faaf43eaf5 100644
> --- a/gcc/gimple-expr.cc
> +++ b/gcc/gimple-expr.cc
> @@ -910,7 +910,8 @@ mark_addressable (tree x)
>      x = TREE_OPERAND (x, 0);
>    while (handled_component_p (x))
>      x = TREE_OPERAND (x, 0);
> -  if (TREE_CODE (x) == MEM_REF
> +  if ((TREE_CODE (x) == MEM_REF
> +       || TREE_CODE (x) == TARGET_MEM_REF)
>        && TREE_CODE (TREE_OPERAND (x, 0)) == ADDR_EXPR)
>      x = TREE_OPERAND (TREE_OPERAND (x, 0), 0);
>    if (!VAR_P (x)

Hi Richard!
Thanks a lot, so great!  This fix works, also pass bootstrap&regtest for
ppc64/ppc64le and x86_64.

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[cvs-commit at gcc dot gnu.org]

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

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:b75f996e846d079251f3a6134617f0405c3ed535

commit r12-7932-gb75f996e846d079251f3a6134617f0405c3ed535
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Mar 31 08:20:43 2022 +0200

    rtl-optimization/105091 - wrong DSE with missed TREE_ADDRESSABLE

    When expanding an aggregate copy into a memcpy call RTL expansion
    uses mark_addressable to ensure the base object is addressable but
    that function doesn't handle TARGET_MEM_REF bases.  Fixed as follows.

    2022-03-31  Richard Biener  <rguenther@suse.de>

            PR rtl-optimization/105091
            * gimple-expr.cc (mark_addressable): Handle TARGET_MEM_REF
            bases.

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.  Btw, this latent issue was likely exposed by r12-910-g35a16e4b38eb9f

[Bug rtl-optimization/105091] RTL dse1 remove stack mem storing incorrectly

[ian at airs dot com]

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

--- Comment #17 from Ian Lance Taylor <ian at airs dot com> ---
Thanks.