public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative
@ 2022-03-10 18:43 jakub at gcc dot gnu.org
  2022-03-10 19:32 ` [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696 jakub at gcc dot gnu.org
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-10 18:43 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 104869
           Summary: [12 Regression] Miscompilation of qt5-qtdeclarative
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
  Target Milestone: ---

Created attachment 52600
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52600&action=edit
qv4codegen.ii.xz

As mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=2061194 ,
qt5-qtdeclarative is miscompiled on powerpc64le-linux with LTO.
I've managed to tweak the preprocessed source, so that the bug is visible also
without lto, but still it isn't a reduced testcase and one can just watch for
the problems in the dumps or assembly.

When the attached preprocessed source is compiled with current trunk on
powerpc64le-linux with -mcpu=power8 -O2 -fvisibility=hidden -fPIC
-fno-exceptions
one can see in the dumps in the
_ZN3QV48Compiler7Codegen5visitEPN6QQmlJS3AST14BreakStatementE function:
optimized dump:
...
  l = OBJ_TYPE_REF(_75;(struct ControlFlow)flow_192->3B) (flow_192, 0,
&D.312010);
  l$generator_225 = l.generator;
  if (l$generator_225 != 0B)
    goto <bb 18>; [5.50%]
  else
    goto <bb 19>; [94.50%]

  <bb 18> [local count: 237749543]:
  target$index_204 = MEM <int> [(struct Label *)&l + 8B];
  l ={v} {CLOBBER(eol)};
  goto <bb 22>; [100.00%]
...
  <bb 22> [local count: 489336362]:
  # target$linkLabel$generator_168 = PHI <l$generator_225(18), flow_83(21)>
  # target$index_81 = PHI <target$index_204(18), -1(21)>
  # target$unwindLevel_73 = PHI <level_193(18), 0(21)>
...
  <bb 27> [local count: 489336362]:
  D.312010 ={v} {CLOBBER};
  D.312010 ={v} {CLOBBER(eol)};
  if (target$linkLabel$generator_168 == 0B)
So far good.
Before fwprop1 it is:
(call_insn 125 124 651 17 (parallel [
            (set (reg:TI 3 3)
                (call (mem:SI (reg:DI 97 ctr) [0
*OBJ_TYPE_REF(_75;flow_192->3B) S4 A8])
                    (const_int 0 [0])))
            (use (const_int 0 [0]))
            (set (reg:DI 2 2)
                (unspec:DI [
                        (const_int 24 [0x18])
                    ] UNSPEC_TOCSLOT))
            (clobber (reg:DI 96 lr))
        ])
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":107:63
769 {*call_value_indirect_elfv2di}
     (expr_list:REG_CALL_DECL (nil)
        (nil))
    (expr_list (use (reg:DI 12 12))
        (expr_list:DI (use (reg:DI 3 3))
            (expr_list:SI (use (reg:DI 4 4))
                (expr_list:DI (use (reg:DI 5 5))
                    (nil))))))
(insn 651 125 652 17 (set (reg:DI 391 [ l ])
        (reg:DI 3 3))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":107:63
670 {*movdi_internal64}
     (nil))
(insn 652 651 127 17 (set (reg:DI 392 [ l+8 ])
        (reg:DI 4 4 [+8 ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":107:63
670 {*movdi_internal64}
     (nil))
(insn 127 652 128 17 (set (reg/f:DI 208 [ l$generator ])
        (reg:DI 391 [ l ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":107:63
670 {*movdi_internal64}
     (nil))
(insn 128 127 129 17 (set (reg:CC 258)
        (compare:CC (reg/f:DI 208 [ l$generator ])
            (const_int 0 [0])))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":108:13
786 {*cmpdi_signed}
     (nil))
(jump_insn 129 128 130 17 (set (pc)
        (if_then_else (eq (reg:CC 258)
                (const_int 0 [0]))
            (label_ref 135)
            (pc)))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":108:13
868 {*cbranch}
     (int_list:REG_BR_PROB 1014686028 (nil))
 -> 135)
...
(note 130 129 131 18 [bb 18] NOTE_INSN_BASIC_BLOCK)
(insn 131 130 132 18 (set (reg:SI 171 [ target$index ])
        (subreg:SI (reg:DI 392 [ l+8 ]) 0))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":109:45
546 {*movsi_internal1}
     (nil))
(insn 132 131 653 18 (clobber (reg:DI 391 [ l ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":109:45
-1
     (nil))
(insn 653 132 6 18 (clobber (reg:DI 392 [ l+8 ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":109:45
-1
     (nil))
(insn 6 653 135 18 (set (reg/v/f:DI 172 [ target$linkLabel$generator ])
        (reg/f:DI 208 [ l$generator ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":107:63
670 {*movdi_internal64}
     (nil))

This still looks good to me.  But next comes fwprop1 and turns that into:
...
(insn 651 125 652 17 (set (reg:DI 391 [ l ])
        (reg:DI 3 3))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":107:63
670 {*movdi_internal64}
     (expr_list:REG_DEAD (reg:DI 3 3)
        (nil)))
(insn 652 651 128 17 (set (reg:DI 392 [ l+8 ])
        (reg:DI 4 4 [+8 ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":107:63
670 {*movdi_internal64}
     (expr_list:REG_DEAD (reg:DI 4 4 [+8 ])
        (nil)))
(insn 128 652 129 17 (set (reg:CC 258)
        (compare:CC (reg:DI 391 [ l ])
            (const_int 0 [0])))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":108:13
786 {*cmpdi_signed}
     (nil))
(jump_insn 129 128 130 17 (set (pc)
        (if_then_else (eq (reg:CC 258)
                (const_int 0 [0]))
            (label_ref 135)
            (pc)))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":108:13
868 {*cbranch}
     (expr_list:REG_DEAD (reg:CC 258)
        (int_list:REG_BR_PROB 1014686028 (nil)))
 -> 135)
# the above part is still fine
...
(note 130 129 131 18 [bb 18] NOTE_INSN_BASIC_BLOCK)
(insn 131 130 132 18 (set (reg:SI 171 [ target$index ])
        (subreg:SI (reg:DI 392 [ l+8 ]) 0))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":109:45
546 {*movsi_internal1}
     (expr_list:REG_DEAD (reg:DI 392 [ l+8 ])
        (nil)))
(insn 132 131 653 18 (clobber (reg:DI 391 [ l ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":109:45
-1
     (expr_list:REG_UNUSED (reg:DI 391 [ l ])
        (nil)))
(insn 653 132 6 18 (clobber (reg:DI 392 [ l+8 ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":109:45
-1
     (expr_list:REG_UNUSED (reg:DI 392 [ l+8 ])
        (nil)))
(insn 6 653 135 18 (set (reg/v/f:DI 172 [ target$linkLabel$generator ])
        (reg:DI 391 [ l ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":107:63
670 {*movdi_internal64}
     (nil))

But this looks wrong to me, clobber on pseudo 391 throws away its value, so
using it in insn 6 looks wrong to me.
The web dump still has the IMHO incorrect:
(note 130 129 131 20 [bb 20] NOTE_INSN_BASIC_BLOCK)
(insn 131 130 132 20 (set (reg:SI 171 [ target$index ])
        (subreg:SI (reg:DI 392 [ l+8 ]) 0))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":109:45
546 {*movsi_internal1}
     (expr_list:REG_DEAD (reg:DI 392 [ l+8 ])
        (nil)))
(insn 132 131 698 20 (clobber (reg:DI 391 [ l ]))
"../../include/QtQml/5.15.3/QtQml/private/../../../../../src/qml/compiler/qv4compilercontrolflow_p.h":109:45
-1
     (nil))
(insn 698 132 135 20 (set (reg:CC 394)
        (compare:CC (reg:DI 403 [ l ])
            (const_int 0 [0]))) 786 {*cmpdi_signed}
     (expr_list:REG_DEAD (reg:DI 391 [ l ])
        (nil)))
(if the clobber wouldn't be there or if fwprop1 didn't forward propagate to the
insns after the clobber the regs clobbered in the clobber, it would be fine).
Next cprop3 removes that clobber (insn 132), but supposedly something wrong is
kept in DF info or where.
Because the init_regs pass later on adds:
(insn 760 131 761 19 (clobber (reg:DI 403 [ l ])) -1
     (nil))
(insn 761 760 698 19 (set (reg:DI 403 [ l ])
        (const_int 0 [0])) -1
     (nil))
before insn 698.  And this remains there until assembly, which contains the
suspicious:
        li 9,0
        mr 31,4
        cmpdi 4,9,0
...
        beq 4,.L7480
Instead of the originally conditional jump (whether
target$linkLabel$generator_168 == 0B) it becomes a fancy unconditional jump,
because constant 0 is loaded into register 9 and then it is compared against 0,
result stored into cr4.

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

* [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696
  2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
@ 2022-03-10 19:32 ` jakub at gcc dot gnu.org
  2022-03-10 19:32 ` jakub at gcc dot gnu.org
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-10 19:32 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |liuhongt at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org
            Summary|[12 Regression]             |[12 Regression]
                   |Miscompilation of           |Miscompilation of
                   |qt5-qtdeclarative           |qt5-qtdeclarative since
                   |                            |r12-6342-ge7a7dbb5ca5dd696

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Bisected to r12-6342-ge7a7dbb5ca5dd69689f1a462ba7620180acfe8b0
Assembly difference is:
--- qv4codegen.s.r12-6341       2022-03-10 20:27:09.392500655 +0100
+++ qv4codegen.s.r12-6342       2022-03-10 20:23:00.256971680 +0100
@@ -52895,9 +52895,11 @@ _ZN3QV48Compiler7Codegen5visitEPN6QQmlJS
        ld 2,24(1)
        mr 9,3
        mr 3,31
-       cmpdi 4,9,0
-       beq 4,.L7388
+       cmpdi 0,9,0
+       beq 0,.L7388
+       li 9,0
        mr 31,4
+       cmpdi 4,9,0
 .L7389:
        ld 3,32(1)
        lwz 9,0(3)
@@ -53428,9 +53430,11 @@ _ZN3QV48Compiler7Codegen5visitEPN6QQmlJS
        ld 2,24(1)
        mr 9,3
        mr 3,31
-       cmpdi 4,9,0
-       beq 4,.L7477
+       cmpdi 0,9,0
+       beq 0,.L7477
+       li 9,0
        mr 31,4
+       cmpdi 4,9,0
 .L7478:
        ld 3,32(1)
        lwz 9,0(3)

That said, I suspect that the r12-6342 change just uncovered a latent fwprop or
rtl-ssa bug, because the commit didn't do anything with propagation across
clobbers.

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

* [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696
  2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
  2022-03-10 19:32 ` [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696 jakub at gcc dot gnu.org
@ 2022-03-10 19:32 ` jakub at gcc dot gnu.org
  2022-03-11 11:00 ` jakub at gcc dot gnu.org
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-10 19:32 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |powerpc64le-linux
   Target Milestone|---                         |12.0
           Priority|P3                          |P1

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

* [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696
  2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
  2022-03-10 19:32 ` [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696 jakub at gcc dot gnu.org
  2022-03-10 19:32 ` jakub at gcc dot gnu.org
@ 2022-03-11 11:00 ` jakub at gcc dot gnu.org
  2022-03-11 12:04 ` jakub at gcc dot gnu.org
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-11 11:00 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-03-11
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Reduced self-contained testcase for the testsuite:

// PR rtl-optimization/104869
// { dg-do run }
// { dg-options "-O2 -fvisibility=hidden" }
// { dg-require-visibility "" }

struct QBasicAtomicInteger {
  [[gnu::noipa]] int loadRelaxed() { return 1; }
};
struct RefCount {
  bool deref() {
    int count = atomic.loadRelaxed();
    if (count)
      return false;
    return deref();
  }
  QBasicAtomicInteger atomic;
};
struct QArrayData {
  RefCount ref;
};
struct QString {
  ~QString();
  QArrayData d;
};
int ok;
QString::~QString() { d.ref.deref(); }
struct Label {
  bool isValid() { return generator; }
  int *generator;
  int index;
};
struct ControlFlow;
struct Codegen {
  [[gnu::noipa]] bool visit();
  ControlFlow *controlFlow;
};
struct ControlFlow {
  enum UnwindType { EE };
  struct UnwindTarget {
    Label linkLabel;
  };
  ControlFlow *parent;
  UnwindType unwindTarget_type;
  UnwindTarget unwindTarget() {
    QString label;
    ControlFlow *flow = this;
    while (flow) {
      Label l = getUnwindTarget(unwindTarget_type, label);
      if (l.isValid())
        return {l};
      flow = flow->parent;
    }
    return UnwindTarget();
  }
  [[gnu::noipa]] Label getUnwindTarget(UnwindType, QString &) {
    Label l = { &ok, 0 };
    return l;
  }
};
[[gnu::noipa]] void foo(int) {
  ok = 1;
}
[[gnu::noipa]] bool Codegen::visit() {
  if (!controlFlow)
    return false;
  ControlFlow::UnwindTarget target = controlFlow->unwindTarget();
  if (target.linkLabel.isValid())
    foo(2);
  return false;
}
int
main() {
  ControlFlow cf = { nullptr, ControlFlow::UnwindType::EE };
  Codegen c = { &cf };
  c.visit();
  if (!ok)
    __builtin_abort ();
}

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

* [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696
  2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2022-03-11 11:00 ` jakub at gcc dot gnu.org
@ 2022-03-11 12:04 ` jakub at gcc dot gnu.org
  2022-03-20 14:02 ` rsandifo at gcc dot gnu.org
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-11 12:04 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I've tried to figure out where the former (GCC 10) all_uses_available_at
checking has gone into (i.e. where we actually check that the propagation is
possible, that something doesn't overwrite or clobber the src from the def_insn
in between the def_insn and use_insn), but I can't find it, must be hidden
somewhere.

Richard, could you please have a look or at least hint at where to look for it?

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

* [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696
  2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2022-03-11 12:04 ` jakub at gcc dot gnu.org
@ 2022-03-20 14:02 ` rsandifo at gcc dot gnu.org
  2022-03-21 10:52 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2022-03-20 14:02 UTC (permalink / raw)
  To: gcc-bugs

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rsandifo at gcc dot gnu.org

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Mine.  The problem is in function_info::make_use_available,
which doesn't cope correctly with cases in which the start
of the BB comes “between” two clobbers (in an RPO sense).

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

* [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696
  2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2022-03-20 14:02 ` rsandifo at gcc dot gnu.org
@ 2022-03-21 10:52 ` cvs-commit at gcc dot gnu.org
  2022-03-21 10:52 ` rsandifo at gcc dot gnu.org
  2022-07-04 19:02 ` cvs-commit at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-03-21 10:52 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:4a3073f04e8b7987ad7bfe1bc23bfeb1d627ee6a

commit r12-7736-g4a3073f04e8b7987ad7bfe1bc23bfeb1d627ee6a
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Mon Mar 21 10:51:56 2022 +0000

    rtl-ssa: Fix prev/next_def confusion [PR104869]

    rtl-ssa chains definitions into an RPO list.  It also groups
    sequences of clobbers together into a single node, so that it's
    possible to skip over the clobbers in constant time in order to
    get the next or previous set.

    When adding a clobber to an insn, the main DF barriers for that
    clobber are the last use of the previous set (if any) and the next
    set (if any); adding a new clobber to a sea of clobbers is fine.
    def_lookup provided the basis for these barriers as prev_def ()
    and next_def ().

    But of course, in hindsight, those were bad names, since they
    implied that the returned values were literally the previous
    definition (of any kind) or the next definition (of any kind).
    And function_info::make_use_available was using the same routines
    assuming that they had that meaning. :-(

    This made a difference for the case where the start of a BB
    occurs in the middle of an (RPO) clobber group: we then want
    the previous and next clobbers in the group, rather than the
    set before the clobber group and the set after the clobber group.

    This patch renames the existing routines to something that's hopefully
    clearer (though also more long-winded).  It then adds routines that
    really do provide the previous and next definitions.

    This complication is supposed to be internal to rtl-ssa and, as
    mentioned above, is part of trying to reduce time complexity.

    gcc/
            PR middle-end/104869
            * rtl-ssa/accesses.h (clobber_group::prev_clobber): Declare.
            (clobber_group::next_clobber): Likewise.
            (def_lookup::prev_def): Rename to...
            (def_lookup::last_def_of_prev_group): ...this.
            (def_lookup::next_def): Rename to...
            (def_lookup::first_def_of_next_group): ...this.
            (def_lookup::matching_or_prev_def): Rename to...
            (def_lookup::matching_set_or_last_def_of_prev_group): ...this.
            (def_lookup::matching_or_next_def): Rename to...
            (def_lookup::matching_set_or_first_def_of_next_group): ...this.
            (def_lookup::prev_def): New function, taking the lookup insn as
            argument.
            (def_lookup::next_def): Likewise.
            * rtl-ssa/member-fns.inl (def_lookup::prev_def): Rename to...
            (def_lookup::last_def_of_prev_group): ...this.
            (def_lookup::next_def): Rename to...
            (def_lookup::first_def_of_next_group): ...this.
            (def_lookup::matching_or_prev_def): Rename to...
            (def_lookup::matching_set_or_last_def_of_prev_group): ...this.
            (def_lookup::matching_or_next_def): Rename to...
            (def_lookup::matching_set_or_first_def_of_next_group): ...this.
            * rtl-ssa/movement.h (restrict_movement_for_dead_range): Update
after
            above renaming.
            * rtl-ssa/accesses.cc (clobber_group::prev_clobber): New function.
            (clobber_group::next_clobber): Likewise.
            (def_lookup::prev_def): Likewise.
            (def_lookup::next_def): Likewise.
            (function_info::make_use_available): Pass the lookup insn to
            def_lookup::prev_def and def_lookup::next_def.

    gcc/testsuite/
            PR middle-end/104869
            * g++.dg/pr104869.C: New test.

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

* [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696
  2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2022-03-21 10:52 ` cvs-commit at gcc dot gnu.org
@ 2022-03-21 10:52 ` rsandifo at gcc dot gnu.org
  2022-07-04 19:02 ` cvs-commit at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2022-03-21 10:52 UTC (permalink / raw)
  To: gcc-bugs

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

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

--- Comment #6 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed.  Thanks for the reduction.

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

* [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696
  2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2022-03-21 10:52 ` rsandifo at gcc dot gnu.org
@ 2022-07-04 19:02 ` cvs-commit at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-07-04 19:02 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:

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

commit r11-10110-gd6a8ac7e92dfe0a51d71525e212147d0b84f1224
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Mon Jul 4 20:02:26 2022 +0100

    rtl-ssa: Fix prev/next_def confusion [PR104869]

    rtl-ssa chains definitions into an RPO list.  It also groups
    sequences of clobbers together into a single node, so that it's
    possible to skip over the clobbers in constant time in order to
    get the next or previous set.

    When adding a clobber to an insn, the main DF barriers for that
    clobber are the last use of the previous set (if any) and the next
    set (if any); adding a new clobber to a sea of clobbers is fine.
    def_lookup provided the basis for these barriers as prev_def ()
    and next_def ().

    But of course, in hindsight, those were bad names, since they
    implied that the returned values were literally the previous
    definition (of any kind) or the next definition (of any kind).
    And function_info::make_use_available was using the same routines
    assuming that they had that meaning. :-(

    This made a difference for the case where the start of a BB
    occurs in the middle of an (RPO) clobber group: we then want
    the previous and next clobbers in the group, rather than the
    set before the clobber group and the set after the clobber group.

    This patch renames the existing routines to something that's hopefully
    clearer (though also more long-winded).  It then adds routines that
    really do provide the previous and next definitions.

    This complication is supposed to be internal to rtl-ssa and, as
    mentioned above, is part of trying to reduce time complexity.

    gcc/
            PR middle-end/104869
            * rtl-ssa/accesses.h (clobber_group::prev_clobber): Declare.
            (clobber_group::next_clobber): Likewise.
            (def_lookup::prev_def): Rename to...
            (def_lookup::last_def_of_prev_group): ...this.
            (def_lookup::next_def): Rename to...
            (def_lookup::first_def_of_next_group): ...this.
            (def_lookup::matching_or_prev_def): Rename to...
            (def_lookup::matching_set_or_last_def_of_prev_group): ...this.
            (def_lookup::matching_or_next_def): Rename to...
            (def_lookup::matching_set_or_first_def_of_next_group): ...this.
            (def_lookup::prev_def): New function, taking the lookup insn as
            argument.
            (def_lookup::next_def): Likewise.
            * rtl-ssa/member-fns.inl (def_lookup::prev_def): Rename to...
            (def_lookup::last_def_of_prev_group): ...this.
            (def_lookup::next_def): Rename to...
            (def_lookup::first_def_of_next_group): ...this.
            (def_lookup::matching_or_prev_def): Rename to...
            (def_lookup::matching_set_or_last_def_of_prev_group): ...this.
            (def_lookup::matching_or_next_def): Rename to...
            (def_lookup::matching_set_or_first_def_of_next_group): ...this.
            * rtl-ssa/movement.h (restrict_movement_for_dead_range): Update
after
            above renaming.
            * rtl-ssa/accesses.cc (clobber_group::prev_clobber): New function.
            (clobber_group::next_clobber): Likewise.
            (def_lookup::prev_def): Likewise.
            (def_lookup::next_def): Likewise.
            (function_info::make_use_available): Pass the lookup insn to
            def_lookup::prev_def and def_lookup::next_def.

    gcc/testsuite/
            PR middle-end/104869
            * g++.dg/pr104869.C: New test.

    (cherry picked from commit 4a3073f04e8b7987ad7bfe1bc23bfeb1d627ee6a)

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

end of thread, other threads:[~2022-07-04 19:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10 18:43 [Bug rtl-optimization/104869] New: [12 Regression] Miscompilation of qt5-qtdeclarative jakub at gcc dot gnu.org
2022-03-10 19:32 ` [Bug rtl-optimization/104869] [12 Regression] Miscompilation of qt5-qtdeclarative since r12-6342-ge7a7dbb5ca5dd696 jakub at gcc dot gnu.org
2022-03-10 19:32 ` jakub at gcc dot gnu.org
2022-03-11 11:00 ` jakub at gcc dot gnu.org
2022-03-11 12:04 ` jakub at gcc dot gnu.org
2022-03-20 14:02 ` rsandifo at gcc dot gnu.org
2022-03-21 10:52 ` cvs-commit at gcc dot gnu.org
2022-03-21 10:52 ` rsandifo at gcc dot gnu.org
2022-07-04 19:02 ` cvs-commit at gcc dot gnu.org

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).