public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3
@ 2021-04-26 10:49 stefansf at linux dot ibm.com
  2021-04-26 14:12 ` [Bug tree-optimization/100263] [11/12 Regression] " jakub at gcc dot gnu.org
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: stefansf at linux dot ibm.com @ 2021-04-26 10:49 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 100263
           Summary: Wrong removal of statement in copyprop3
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: stefansf at linux dot ibm.com
  Target Milestone: ---
            Target: s390*-*-*

Created attachment 50676
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50676&action=edit
copyprop3

int a = 3, d, l, o, p, q;
unsigned char b, f, g;
unsigned char c[9][9][3];
long e;
unsigned h;
short j, k;
static unsigned char r(int s) {
  for (; b-2; b=b-2) {
    int *m = &d;
    if (a) {
      char *n = &c[3][8][2];
      *n = 1;
      for (; h;)
        for (; g;)
          for (; e;)
            ;
    } else {
      int i = 0;
      for (; i < 2; i++)
        if (*m)
          return 0;
      if (s)
        l = k |= ((j-- != b) <= s) - (long)s;
      else
        return f;
    }
  }
  return 0;
}
int main() {
  r(b);
  if (c[3][8][2] != 1)
    __builtin_abort ();
}

The outermost loop is executed 127 times. Since variable `a` does not change
from its initial value 3, the store to `c[3][8][2]` must materialize since the
infinite loops over variables h, g, e are never executed. However, running `gcc
-march=z13 t.c -O1 && ./a.out` results in an abort. Runs are successfull if
using different optimization levels than 1.

Prior to copyprop3 we have

<bb 3>
c__lsm.20_74 = MEM[(char *)&c + 107B];

<bb 4>
if (a.1_7 != 0)
  goto <bb 5>; [50.00%]
else
  goto <bb 59>; [50.00%]

<bb 5> [local count: 5160584476]:
c__lsm.20_94 = 1;
c__lsm_flag.21_95 = 1;

whereas afterwards the store to `c__lsm.20_94` is removed. Dump file contains:

Removing dead stmt c__lsm.20_94 = 1;

In total we have that the only store to `c[3][8][2]` happens in inner loop over
variable `h` which is never executed. There we have `MEM[(char *)&c + 107B] =
1;`.

Tested against g:3971aee9dd8d6323c377d1b241173f7d2b51a835 on IBM Z where it
fails. Runs successfully on x86 (by chance?).
Bisection stops at g:f9e1ea10e657af9fb02fafecf1a600740fd34409 which might not
be the root cause.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/100263] [11/12 Regression] Wrong removal of statement in copyprop3
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
@ 2021-04-26 14:12 ` jakub at gcc dot gnu.org
  2021-04-26 15:37 ` jakub at gcc dot gnu.org
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-26 14:12 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-04-26
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
            Summary|Wrong removal of statement  |[11/12 Regression] Wrong
                   |in copyprop3                |removal of statement in
                   |                            |copyprop3
   Target Milestone|---                         |11.2
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
           Priority|P3                          |P2

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
In can confirm it is r11-39-gf9e1ea10e657af9fb02fafecf1a600740fd34409
Before lim2 the dump is identical, lim2 dump difference between r11-38 and
r11-39
is:
--- pr100263.c.131t.lim2.r11-38       2021-04-26 15:54:38.623719858 +0200
+++ pr100263.c.131t.lim2.r11-39        2021-04-26 15:54:53.982545411 +0200
@@ -165,26 +165,23 @@ main ()
   j_lsm_flag.23_69 = 0;
   k_lsm.24_70 = k;
   k_lsm_flag.25_71 = 0;
-  l_lsm.26_72 = l;
-  l_lsm_flag.27_73 = 0;
-  c__lsm.20_74 = MEM[(char *)&c + 107B];
-  c__lsm_flag.21_75 = 0;
+  l_lsm_flag.27_72 = 0;
+  c__lsm_flag.21_73 = 0;
   h.4_5 = h;
-  b_lsm.28_76 = b;
-  b_lsm_flag.29_77 = 0;
+  b_lsm_flag.29_74 = 0;

   <bb 3> [local count: 10321168952]:
   # b.12_44 = PHI <b.0_1(18), _32(23)>
-  # c__lsm.20_50 = PHI <c__lsm.20_74(18), c__lsm.20_51(23)>
-  # c__lsm_flag.21_52 = PHI <c__lsm_flag.21_75(18), c__lsm_flag.21_53(23)>
+  # c__lsm.20_50 = PHI <c__lsm.20_75(D)(18), c__lsm.20_51(23)>
+  # c__lsm_flag.21_52 = PHI <c__lsm_flag.21_73(18), c__lsm_flag.21_53(23)>
   # j_lsm.22_54 = PHI <j_lsm.22_68(18), j_lsm.22_55(23)>
   # j_lsm_flag.23_56 = PHI <j_lsm_flag.23_69(18), j_lsm_flag.23_57(23)>
   # k_lsm.24_58 = PHI <k_lsm.24_70(18), k_lsm.24_59(23)>
   # k_lsm_flag.25_60 = PHI <k_lsm_flag.25_71(18), k_lsm_flag.25_61(23)>
-  # l_lsm.26_62 = PHI <l_lsm.26_72(18), l_lsm.26_63(23)>
-  # l_lsm_flag.27_64 = PHI <l_lsm_flag.27_73(18), l_lsm_flag.27_65(23)>
-  # b_lsm.28_66 = PHI <b_lsm.28_76(18), b_lsm.28_101(23)>
-  # b_lsm_flag.29_67 = PHI <b_lsm_flag.29_77(18), b_lsm_flag.29_102(23)>
+  # l_lsm.26_62 = PHI <l_lsm.26_76(D)(18), l_lsm.26_63(23)>
+  # l_lsm_flag.27_64 = PHI <l_lsm_flag.27_72(18), l_lsm_flag.27_65(23)>
+  # b_lsm.28_66 = PHI <b_lsm.28_77(D)(18), b_lsm.28_101(23)>
+  # b_lsm_flag.29_67 = PHI <b_lsm_flag.29_74(18), b_lsm_flag.29_102(23)>
   if (a.1_7 != 0)
     goto <bb 4>; [50.00%]
   else

I can reproduce the same problem even when replacing
      char *n = &c[3][8][2];
      *n = 1;
with
      c[3][8][2] = 1;

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/100263] [11/12 Regression] Wrong removal of statement in copyprop3
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
  2021-04-26 14:12 ` [Bug tree-optimization/100263] [11/12 Regression] " jakub at gcc dot gnu.org
@ 2021-04-26 15:37 ` jakub at gcc dot gnu.org
  2021-04-26 16:50 ` jakub at gcc dot gnu.org
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-26 15:37 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Though, looking at the r11-39 optimized dump, I actually don't see anything
wrong.
The path taken in the testcase should be:
   <bb 2> [local count: 1073741824]:
   b.0_1 = b;
   if (b.0_1 != 2)
     goto <bb 3>; [96.34%]
...
   <bb 3> [local count: 1034442872]:
...
   <bb 4> [local count: 10321168952]:
   # b.12_44 = PHI <b.0_1(3), _32(42)>
   # c__lsm.20_50 = PHI <c__lsm.20_75(D)(3), c__lsm.20_51(42)>
   # c__lsm_flag.21_52 = PHI <0(3), c__lsm_flag.21_53(42)>
...
   if (a.1_7 != 0)
     goto <bb 5>; [50.00%]
   else
     goto <bb 19>; [50.00%]
...
   <bb 5> [local count: 5160584476]:
   if (h.4_5 != 0)
     goto <bb 9>; [89.00%]
   else
     goto <bb 42>; [11.00%]
...
   <bb 42> [local count: 9639533214]:
   # c__lsm.20_51 = PHI <c__lsm.20_50(41), 1(5)>
   # c__lsm_flag.21_53 = PHI <c__lsm_flag.21_52(41), 1(5)>
...
   _32 = b.12_44 + 254;
   if (_32 != 2)
     goto <bb 4>; [96.34%]
   else
     goto <bb 43>; [3.66%]

   <bb 43> [local count: 352806927]:
   if (c__lsm_flag.21_53 != 0)
     goto <bb 44>; [66.67%]
   else
     goto <bb 45>; [33.33%]

   <bb 44> [local count: 235204617]:
   MEM[(char *)&c + 107B] = c__lsm.20_51;
where bb will loop 126 times and then fall through to bb 43, but when bb 42 is
reached from bb 5 (should be always), then both SSA_NAMEs 51 and 53 are 1 and
so
the store of 1 should happen.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/100263] [11/12 Regression] Wrong removal of statement in copyprop3
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
  2021-04-26 14:12 ` [Bug tree-optimization/100263] [11/12 Regression] " jakub at gcc dot gnu.org
  2021-04-26 15:37 ` jakub at gcc dot gnu.org
@ 2021-04-26 16:50 ` jakub at gcc dot gnu.org
  2021-04-27  5:56 ` [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop stefansf at linux dot ibm.com
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-26 16:50 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And, just to verify it, I've compiled the testcase with r11-39 with a
breakpoint on pass_expand::execute and set global_options.x_optimize = 0 there,
and resulting testcase passes.
So most likely things go wrong during RTL optimizations.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (2 preceding siblings ...)
  2021-04-26 16:50 ` jakub at gcc dot gnu.org
@ 2021-04-27  5:56 ` stefansf at linux dot ibm.com
  2021-04-27 10:27 ` stefansf at linux dot ibm.com
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: stefansf at linux dot ibm.com @ 2021-04-27  5:56 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
You are right. I got lured by the fact that the assignments c__lsm.20_94 = 1;
and c__lsm_flag.21_95 = 1; of bb5 are "moved" into the PHI as e.g.

   # c__lsm.20_51 = PHI <c__lsm.20_50(41), 1(5)>
   # c__lsm_flag.21_53 = PHI <c__lsm_flag.21_52(41), 1(5)>

I will have a look at the RTL output then.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (3 preceding siblings ...)
  2021-04-27  5:56 ` [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop stefansf at linux dot ibm.com
@ 2021-04-27 10:27 ` stefansf at linux dot ibm.com
  2021-04-28  8:43 ` stefansf at linux dot ibm.com
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: stefansf at linux dot ibm.com @ 2021-04-27 10:27 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
It looks like a mode mismatch:

(insn 201 200 378 3 (set (reg:DI 17 %f2 [196])
        (const_int 1 [0x1])) "t.c":23:36 1467 {*movdi_64}
     (expr_list:REG_EQUIV (const_int 1 [0x1])
        (nil)))
...
(insn 312 44 313 4 (set (reg:QI 5 %r5 [orig:74 c__lsm_flag.21 ] [74])
        (reg:QI 17 %f2 [orig:198 l_lsm_flag.27 ] [198])) "t.c":13:14 1480
{*movqi}
     (nil))
...
(insn 245 244 246 41 (set (reg:SI 5 %r5 [orig:169 c__lsm_flag.21+-3 ] [169])
        (zero_extend:SI (reg:QI 5 %r5 [orig:74 c__lsm_flag.21 ] [74]))) 1652
{*zero_extendqisi2_extimm}
     (nil))
(note 246 245 247 41 NOTE_INSN_DELETED)
(jump_insn 247 246 248 41 (parallel [                                           
            (set (pc)
                (if_then_else (eq (reg:SI 5 %r5 [orig:169 c__lsm_flag.21+-3 ]
[169])
                        (const_int 0 [0]))
                    (label_ref 251)
                    (pc)))
            (clobber (reg:CC 33 %cc))
        ]) 1458 {*cmp_and_br_signed_si}
     (expr_list:REG_DEAD (reg:SI 5 %r5 [orig:169 c__lsm_flag.21+-3 ] [169])
        (expr_list:REG_UNUSED (reg:CC 33 %cc)
            (int_list:REG_BR_PROB 357913950 (nil))))
 -> 251)
(note 248 247 249 42 [bb 42] NOTE_INSN_BASIC_BLOCK)
(insn 249 248 250 42 (set (reg/f:DI 1 %r1 [170])
        (symbol_ref:DI ("*.LANCHOR0") [flags 0x182])) 1467 {*movdi_64}
     (expr_list:REG_EQUIV (symbol_ref:DI ("*.LANCHOR0") [flags 0x182])
        (nil)))
(insn 250 249 251 42 (set (mem/c:QI (plus:DI (reg/f:DI 1 %r1 [170])
                (const_int 135 [0x87])) [0 MEM[(char *)&c + 107B]+0 S1 A8])
        (reg:QI 18 %f4 [orig:73 D.2339 ] [73])) 1480 {*movqi}
     (expr_list:REG_DEAD (reg:QI 18 %f4 [orig:73 D.2339 ] [73])
        (expr_list:REG_DEAD (reg/f:DI 1 %r1 [170])
            (nil))))

Register f2 is written to in DI mode and read from in QI mode. The final
assembler for the read is `vlgvb %r5,%v2,0`. Inspecting v2 via GDB we have:

  v2_int64 = {0x1, 0x0}

which means r5 is zero afterwards and therefore the condition r5==0 is always
true so the store from insn 250 never happens.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (4 preceding siblings ...)
  2021-04-27 10:27 ` stefansf at linux dot ibm.com
@ 2021-04-28  8:43 ` stefansf at linux dot ibm.com
  2021-04-28 10:03 ` jakub at gcc dot gnu.org
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: stefansf at linux dot ibm.com @ 2021-04-28  8:43 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
Prior postreload we have

(insn 12 379 332 3 (set (reg:QI 17 %f2 [orig:198 l_lsm_flag.27 ] [198])
        (const_int 1 [0x1])) 1480 {*movqi}
     (expr_list:REG_EQUIV (const_int 1 [0x1])
        (nil)))

which gets substituted during postreload by

(insn 12 379 332 3 (set (reg:QI 17 %f2 [orig:198 l_lsm_flag.27 ] [198])
        (reg:QI 17 %f2 [orig:198 l_lsm_flag.27 ] [198])) 1480 {*movqi}
     (expr_list:REG_EQUIV (const_int 1 [0x1])
        (nil)))

which gets deleted during split2 where we have

  deleting insn with uid = 12.

The culprit seems to be that postreload changes the RHS of the assignment to
`reg:QI 17 %f2` which is wrong. Register %f2 holds indeed constant 1, however,
in DImode which is not compatible to QImode on IBM Z.

The decision takes place in function move2add_valid_value_p where the wrong
offset is returned by call

  subreg_regno_offset (17, DImode, 7, QImode)

I would have expected offset 7 but 0 is returned which renders the subsequent
if-condition false. The 0 comes from function call subreg_get_info which
returns in this case via

/* Lowpart subregs are otherwise valid.  */
if (!rknown && known_eq (offset, subreg_lowpart_offset (ymode, xmode)))
  {
    info->representable_p = true;
    rknown = true;

    if (known_eq (offset, 0U) || nregs_xmode == nregs_ymode)
      {
        info->offset = 0;
        info->nregs = nregs_ymode;
        return;
      }
  }

The offset doesn't equal zero but the number of registers. Still the offset is
set to zero.

I did a quick test by using instead

diff --git a/gcc/postreload.c b/gcc/postreload.c
index dc67643384d..64297be2c45 100644
--- a/gcc/postreload.c
+++ b/gcc/postreload.c
@@ -1732,12 +1732,7 @@ move2add_valid_value_p (int regno, scalar_int_mode mode)
         (REG:reg_mode[regno] regno).  Now, for big endian, the starting
         regno of the lowpart might be different.  */
       poly_int64 s_off = subreg_lowpart_offset (mode, old_mode);
-      s_off = subreg_regno_offset (regno, old_mode, s_off, mode);
-      if (maybe_ne (s_off, 0))
-       /* We could in principle adjust regno, check reg_mode[regno] to be
-          BLKmode, and return s_off to the caller (vs. -1 for failure),
-          but we currently have no callers that could make use of this
-          information.  */
+      if (simplify_subreg_regno (regno, old_mode, s_off, mode) < 0)
        return false;
     }

which works at least for the example (haven't done a bootstrap nor regtest
yet). However, I'm still wondering whether subreg_get_info is supposed to
return with a zero offset in cases like this? Any thoughts?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (5 preceding siblings ...)
  2021-04-28  8:43 ` stefansf at linux dot ibm.com
@ 2021-04-28 10:03 ` jakub at gcc dot gnu.org
  2021-04-28 10:41 ` ebotcazou at gcc dot gnu.org
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-28 10:03 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amylaar at gcc dot gnu.org,
                   |                            |ebotcazou at gcc dot gnu.org

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The code in question in postreload.c has been added for PR57439.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (6 preceding siblings ...)
  2021-04-28 10:03 ` jakub at gcc dot gnu.org
@ 2021-04-28 10:41 ` ebotcazou at gcc dot gnu.org
  2021-04-28 11:52 ` stefansf at linux dot ibm.com
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2021-04-28 10:41 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I did a quick test by using instead
> 
> diff --git a/gcc/postreload.c b/gcc/postreload.c
> index dc67643384d..64297be2c45 100644
> --- a/gcc/postreload.c
> +++ b/gcc/postreload.c
> @@ -1732,12 +1732,7 @@ move2add_valid_value_p (int regno, scalar_int_mode
> mode)
>          (REG:reg_mode[regno] regno).  Now, for big endian, the starting
>          regno of the lowpart might be different.  */
>        poly_int64 s_off = subreg_lowpart_offset (mode, old_mode);
> -      s_off = subreg_regno_offset (regno, old_mode, s_off, mode);
> -      if (maybe_ne (s_off, 0))
> -       /* We could in principle adjust regno, check reg_mode[regno] to be
> -          BLKmode, and return s_off to the caller (vs. -1 for failure),
> -          but we currently have no callers that could make use of this
> -          information.  */
> +      if (simplify_subreg_regno (regno, old_mode, s_off, mode) < 0)
>         return false;
>      }
> 
> which works at least for the example (haven't done a bootstrap nor regtest
> yet). However, I'm still wondering whether subreg_get_info is supposed to
> return with a zero offset in cases like this? Any thoughts?

See the head comment of subreg_get_info, especially at the end.  We still need
to check that the regno offset is zero since we reuse the same reg, but we also
need to check targetm.hard_regno_mode_ok (regno, mode) here.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (7 preceding siblings ...)
  2021-04-28 10:41 ` ebotcazou at gcc dot gnu.org
@ 2021-04-28 11:52 ` stefansf at linux dot ibm.com
  2021-04-28 12:01 ` ebotcazou at gcc dot gnu.org
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: stefansf at linux dot ibm.com @ 2021-04-28 11:52 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
Shouldn't we rather check for REG_CAN_CHANGE_MODE_P? A check for
TARGET_HARD_REGNO_MODE_OK for a FP register and QImode is successful.

Using the following also fixes the test for me:

diff --git a/gcc/postreload.c b/gcc/postreload.c
index dc67643384d..3dccbe63cf4 100644
--- a/gcc/postreload.c
+++ b/gcc/postreload.c
@@ -1733,7 +1733,7 @@ move2add_valid_value_p (int regno, scalar_int_mode mode)
         regno of the lowpart might be different.  */
       poly_int64 s_off = subreg_lowpart_offset (mode, old_mode);
       s_off = subreg_regno_offset (regno, old_mode, s_off, mode);
-      if (maybe_ne (s_off, 0))
+      if (maybe_ne (s_off, 0) || !REG_CAN_CHANGE_MODE_P (regno, old_mode,
mode))
        /* We could in principle adjust regno, check reg_mode[regno] to be
           BLKmode, and return s_off to the caller (vs. -1 for failure),
           but we currently have no callers that could make use of this

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (8 preceding siblings ...)
  2021-04-28 11:52 ` stefansf at linux dot ibm.com
@ 2021-04-28 12:01 ` ebotcazou at gcc dot gnu.org
  2021-04-28 14:20 ` stefansf at linux dot ibm.com
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2021-04-28 12:01 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Shouldn't we rather check for REG_CAN_CHANGE_MODE_P? A check for
> TARGET_HARD_REGNO_MODE_OK for a FP register and QImode is successful.

OK, then it's probably better to add it to:

      if (!is_a <scalar_int_mode> (reg_mode[regno], &old_mode)
          || !MODES_OK_FOR_MOVE2ADD (mode, old_mode))
        return false;

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (9 preceding siblings ...)
  2021-04-28 12:01 ` ebotcazou at gcc dot gnu.org
@ 2021-04-28 14:20 ` stefansf at linux dot ibm.com
  2021-05-05 15:01 ` cvs-commit at gcc dot gnu.org
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: stefansf at linux dot ibm.com @ 2021-04-28 14:20 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
(In reply to Eric Botcazou from comment #10)
> OK, then it's probably better to add it to:
> 
>       if (!is_a <scalar_int_mode> (reg_mode[regno], &old_mode)
> 	  || !MODES_OK_FOR_MOVE2ADD (mode, old_mode))
> 	return false;

Ok, I will move the check up there. Currently running bootstrap+regtest on x86
as well as IBM Z.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (10 preceding siblings ...)
  2021-04-28 14:20 ` stefansf at linux dot ibm.com
@ 2021-05-05 15:01 ` cvs-commit at gcc dot gnu.org
  2021-05-05 15:07 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-05-05 15:01 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Stefan Schulze Frielinghaus
<stefansf@gcc.gnu.org>:

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

commit r12-518-gbb283170e7a1f39bf533651418daf10ad18eccfc
Author: Stefan Schulze Frielinghaus <stefansf@linux.ibm.com>
Date:   Wed May 5 17:00:43 2021 +0200

    PR rtl-optimization/100263: Ensure register can change mode

    For move2add_valid_value_p we also have to ask the target whether a
    register can be accessed in a different mode than it was set before.

    gcc/ChangeLog:

            PR rtl-optimization/100263
            * postreload.c (move2add_valid_value_p): Ensure register can
            change mode.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (11 preceding siblings ...)
  2021-05-05 15:01 ` cvs-commit at gcc dot gnu.org
@ 2021-05-05 15:07 ` cvs-commit at gcc dot gnu.org
  2021-05-05 15:10 ` cvs-commit at gcc dot gnu.org
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-05-05 15:07 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Stefan Schulze Frielinghaus
<stefansf@gcc.gnu.org>:

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

commit r11-8355-gfcad2894215879b740dce74e72247b6efa326397
Author: Stefan Schulze Frielinghaus <stefansf@linux.ibm.com>
Date:   Wed May 5 17:06:52 2021 +0200

    PR rtl-optimization/100263: Ensure register can change mode

    For move2add_valid_value_p we also have to ask the target whether a
    register can be accessed in a different mode than it was set before.

    gcc/ChangeLog:

            PR rtl-optimization/100263
            * postreload.c (move2add_valid_value_p): Ensure register can
            change mode.

    (cherry picked from commit bb283170e7a1f39bf533651418daf10ad18eccfc)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (12 preceding siblings ...)
  2021-05-05 15:07 ` cvs-commit at gcc dot gnu.org
@ 2021-05-05 15:10 ` cvs-commit at gcc dot gnu.org
  2021-05-05 15:11 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-05-05 15:10 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Stefan Schulze Frielinghaus
<stefansf@gcc.gnu.org>:

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

commit r10-9801-ga3a6a7f0dcd3e0775c700b4a06320ac911a2c5b5
Author: Stefan Schulze Frielinghaus <stefansf@linux.ibm.com>
Date:   Wed May 5 17:10:15 2021 +0200

    PR rtl-optimization/100263: Ensure register can change mode

    For move2add_valid_value_p we also have to ask the target whether a
    register can be accessed in a different mode than it was set before.

    gcc/ChangeLog:

            PR rtl-optimization/100263
            * postreload.c (move2add_valid_value_p): Ensure register can
            change mode.

    (cherry picked from commit bb283170e7a1f39bf533651418daf10ad18eccfc)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (13 preceding siblings ...)
  2021-05-05 15:10 ` cvs-commit at gcc dot gnu.org
@ 2021-05-05 15:11 ` cvs-commit at gcc dot gnu.org
  2021-05-05 15:13 ` cvs-commit at gcc dot gnu.org
  2021-05-06 12:51 ` stefansf at linux dot ibm.com
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-05-05 15:11 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Stefan Schulze Frielinghaus
<stefansf@gcc.gnu.org>:

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

commit r9-9513-gd7bd91c7e059ee24bcf991d503bcd9856618a670
Author: Stefan Schulze Frielinghaus <stefansf@linux.ibm.com>
Date:   Wed May 5 17:11:24 2021 +0200

    PR rtl-optimization/100263: Ensure register can change mode

    For move2add_valid_value_p we also have to ask the target whether a
    register can be accessed in a different mode than it was set before.

    gcc/ChangeLog:

            PR rtl-optimization/100263
            * postreload.c (move2add_valid_value_p): Ensure register can
            change mode.

    (cherry picked from commit bb283170e7a1f39bf533651418daf10ad18eccfc)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (14 preceding siblings ...)
  2021-05-05 15:11 ` cvs-commit at gcc dot gnu.org
@ 2021-05-05 15:13 ` cvs-commit at gcc dot gnu.org
  2021-05-06 12:51 ` stefansf at linux dot ibm.com
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-05-05 15:13 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Stefan Schulze Frielinghaus
<stefansf@gcc.gnu.org>:

https://gcc.gnu.org/g:06d75273ff7bb7af72ae83abef858c079245b602

commit r8-10952-g06d75273ff7bb7af72ae83abef858c079245b602
Author: Stefan Schulze Frielinghaus <stefansf@linux.ibm.com>
Date:   Wed May 5 17:12:35 2021 +0200

    PR rtl-optimization/100263: Ensure register can change mode

    For move2add_valid_value_p we also have to ask the target whether a
    register can be accessed in a different mode than it was set before.

    gcc/ChangeLog:

            PR rtl-optimization/100263
            * postreload.c (move2add_valid_value_p): Ensure register can
            change mode.

    (cherry picked from commit bb283170e7a1f39bf533651418daf10ad18eccfc)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop
  2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
                   ` (15 preceding siblings ...)
  2021-05-05 15:13 ` cvs-commit at gcc dot gnu.org
@ 2021-05-06 12:51 ` stefansf at linux dot ibm.com
  16 siblings, 0 replies; 18+ messages in thread
From: stefansf at linux dot ibm.com @ 2021-05-06 12:51 UTC (permalink / raw)
  To: gcc-bugs

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

Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> changed:

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

--- Comment #17 from Stefan Schulze Frielinghaus <stefansf at linux dot ibm.com> ---
Closing since fixed in releases/gcc-{8,9,10,11} and mainline.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-05-06 12:51 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26 10:49 [Bug tree-optimization/100263] New: Wrong removal of statement in copyprop3 stefansf at linux dot ibm.com
2021-04-26 14:12 ` [Bug tree-optimization/100263] [11/12 Regression] " jakub at gcc dot gnu.org
2021-04-26 15:37 ` jakub at gcc dot gnu.org
2021-04-26 16:50 ` jakub at gcc dot gnu.org
2021-04-27  5:56 ` [Bug rtl-optimization/100263] [11/12 Regression] RTL optimizers miscompile loop stefansf at linux dot ibm.com
2021-04-27 10:27 ` stefansf at linux dot ibm.com
2021-04-28  8:43 ` stefansf at linux dot ibm.com
2021-04-28 10:03 ` jakub at gcc dot gnu.org
2021-04-28 10:41 ` ebotcazou at gcc dot gnu.org
2021-04-28 11:52 ` stefansf at linux dot ibm.com
2021-04-28 12:01 ` ebotcazou at gcc dot gnu.org
2021-04-28 14:20 ` stefansf at linux dot ibm.com
2021-05-05 15:01 ` cvs-commit at gcc dot gnu.org
2021-05-05 15:07 ` cvs-commit at gcc dot gnu.org
2021-05-05 15:10 ` cvs-commit at gcc dot gnu.org
2021-05-05 15:11 ` cvs-commit at gcc dot gnu.org
2021-05-05 15:13 ` cvs-commit at gcc dot gnu.org
2021-05-06 12:51 ` stefansf at linux dot ibm.com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).