[Bug rtl-optimization/62146] New: CSE replaces constant with an expression incorrectly

[Bug rtl-optimization/62146] New: CSE replaces constant with an expression incorrectly

[eraman at google dot com]

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

            Bug ID: 62146
           Summary: CSE replaces constant with an expression incorrectly
           Product: gcc
           Version: 4.9.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: eraman at google dot com

Created attachment 33330
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33330&action=edit
preprocessed test case obtained using creduce

$ ./g++_4_9 --version
g++_4_9 (GCC) 4.9.2 20140814 (prerelease)

$ ./g++_4_9     test.ii -std=gnu++11        -O2 -S

Looking at the test.s, we see this following fragment in _ZN2C13fooEv:

.L8:
        xorl    %esi, %esi
        testq   %rbp, %rbp
        je      .L7
        movl    $16, %edi
        call    _Znwm
        movq    $_ZTV1GIN2C19TokenTypeEE+16, (%rax)
        movq    $_ZN2C19TokenType8AddTokenEv, 8(%rax)
        movq    %rax, %rsi


The _Znwm corresponds to new G < CL > ( 0) in test.ii:22 Once the object is
allocated, the code stores $_ZN2C19TokenType8AddTokenEv at offset 8. This is
incorrect - it should store 0 there. Codegen is fine at -O1. Corresponding code
with -O1:

.L9:
        movl    $0, %esi
        testq   %rbp, %rbp
        je      .L8
        movl    $16, %edi
        call    _Znwm
        movq    $_ZTV1GIN2C19TokenTypeEE+16, (%rax)
        movq    $0, 8(%rax)
        movq    %rax, %rsi

There seems to be multiple issues here. I am not able to reproduce the issue in
trunk, but I suspect it is simply hidden by some other transformation.

The first CSE pass incorrectly replaces 0 with _ZTV1GIN2C19TokenTypeEE. We have
this code before rtl-cse1:
...
(insn 18 17 19 4 (set (reg/f:DI 90)
        (symbol_ref/i:DI ("_ZN2C19TokenType8AddTokenEv") [flags 0x1] 
<function_decl 0x7fdccd539700 AddToken>)) test.ii:22 89 {*movdi_internal}
     (nil))
(insn 19 18 20 4 (set (reg:CCZ 17 flags)
        (compare:CCZ (reg/f:DI 90)
            (const_int 0 [0]))) test.ii:22 4 {*cmpdi_ccno_1}
     (nil))
(jump_insn 20 19 21 4 (set (pc)
        (if_then_else (eq (reg:CCZ 17 flags)
                (const_int 0 [0]))
            (label_ref:DI 48)
            (pc))) test.ii:22 596 {*jcc_1}
     (int_list:REG_BR_PROB 2165 (nil))
 -> 48)
...
(code_label 48 8 47 6 9 "" [1 uses])
(note 47 48 7 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
(insn 7 47 27 6 (set (reg/f:DI 88 [ D.2494 ])
        (const_int 0 [0])) test.ii:22 89 {*movdi_internal}
     (nil))

In this EBB, CSE first looks at insn 18 and concludes reg 90 and
symbol_ref/i:DI ("_ZN2C19TokenType8AddTokenEv") are equal. Then it looks at the
branch and concludes that reg 90 has to be 0 in BB 6 and puts 0 also in the
same euqivalent class. Then it looks at insn 7 and replaces the const_int 0
with reference to the _ZTV1GIN2C19TokenTypeEE symbol. This shouldn't happen. 

Next, PRE commons out this symbol into a register and puts a REG_EQUAL note on
this insn to say the src reg is the equal to _ZN2C19TokenType8AddTokenEv.

(insn 52 47 27 6 (set (reg/f:DI 88 [ D.2494 ])
        (reg/f:DI 91 [ D.2494 ])) test.ii:22 -1
     (expr_list:REG_EQUAL (symbol_ref/i:DI ("_ZN2C19TokenType8AddTokenEv")
[flags 0x1]  <function_decl 0x7fdccd539700 AddToken>)
        (nil)))


 rtl-cprop2 then replaces the register 91 back with 0. However it leaves the
REG_EQUAL note on that insn 52. 

So far, this shouldn't affect anything as all these happen in a dead BB (the
branch above is never taken). However, CE hoists insn 52 above the branch:

(insn 52 17 19 4 (set (reg/f:DI 88 [ D.2494 ])
        (const_int 0 [0])) test.ii:22 89 {*movdi_internal}
     (expr_list:REG_EQUAL (symbol_ref/i:DI ("_ZN2C19TokenType8AddTokenEv")
[flags 0x1]  <function_decl 0x7fdccd539700 AddToken>)
        (nil)))
(insn 19 52 20 4 (set (reg:CCZ 17 flags)
        (compare:CCZ (reg/f:DI 91 [ D.2494 ])
            (const_int 0 [0]))) test.ii:22 4 {*cmpdi_ccno_1}
     (expr_list:REG_EQUAL (compare:CCZ (symbol_ref/i:DI
("_ZN2C19TokenType8AddTokenEv") [flags 0x1]  <function_decl 0x7fdccd539700
AddToken>)
            (const_int 0 [0]))
        (nil)))
(jump_insn 20 19 21 4 (set (pc)
        (if_then_else (eq (reg:CCZ 17 flags)
                (const_int 0 [0]))
            (label_ref:DI 27)
            (pc))) test.ii:22 596 {*jcc_1}
     (expr_list:REG_DEAD (reg:CCZ 17 flags)
        (int_list:REG_BR_PROB 2165 (nil)))
 -> 27)


Now, a later CSE pass (rtl-cse2) looks at the EBB containing the above jump
(insn 19) and the *fallthru* and thinks that 0 and _ZN2C19TokenType8AddTokenEv
are equal based on the REG_EQUAL note. It changes the following insn in the
fallthru

(insn 26 25 8 7 (set (mem/f:DI (plus:DI (reg/f:DI 87 [ D.2493 ])
                (const_int 8 [0x8])) [4 MEM[(struct G *)_13].member_+0 S8 A64])
        (const_int 0 [0])) test.ii:10 89 {*movdi_internal}
     (nil))

to

(insn 26 25 8 7 (set (mem/f:DI (plus:DI (reg/f:DI 87 [ D.2493 ])
                (const_int 8 [0x8])) [4 MEM[(struct G *)_13].member_+0 S8 A64])
        (symbol_ref/i:DI ("_ZN2C19TokenType8AddTokenEv") [flags 0x1] 
<function_decl 0x7fdccd539700 AddToken>)) test.ii:10 89 {*movdi_internal}
     (nil))

[Bug rtl-optimization/62146] CSE replaces constant with an expression incorrectly

[eraman at google dot com]

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

--- Comment #1 from Easwaran Raman <eraman at google dot com> ---
Created attachment 33331
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33331&action=edit
Patch to remove obsolete REG_EQUAL note during RTL copy prop

This patch kills the REG_EQUAL note during cprop when the propagated value is a
constant. This doesn't the CSE problem that conflates 0 and the symbol.

[Bug rtl-optimization/62146] CSE replaces constant with an expression incorrectly

[eraman at gcc dot gnu.org]

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

--- Comment #2 from eraman at gcc dot gnu.org ---
Author: eraman
Date: Fri Sep  5 22:23:26 2014
New Revision: 214977

URL: https://gcc.gnu.org/viewcvs?rev=214977&root=gcc&view=rev
Log:
2014-09-05  Easwaran Raman  <eraman@google.com>

        PR rtl-optimization/62146
        * ifcvt.c (dead_or_predicable): Make removal of REG_EQUAL note of
          hoisted instruction unconditional.

        testsuite:
        * testsuite/g++.dg/opt/pr62146.C: New.

Added:
    trunk/gcc/testsuite/g++.dg/opt/pr62146.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ifcvt.c
    trunk/gcc/testsuite/ChangeLog

[Bug rtl-optimization/62146] CSE replaces constant with an expression incorrectly

[eraman at gcc dot gnu.org]

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

--- Comment #3 from eraman at gcc dot gnu.org ---
Author: eraman
Date: Sat Sep  6 01:49:07 2014
New Revision: 214981

URL: https://gcc.gnu.org/viewcvs?rev=214981&root=gcc&view=rev
Log:
2014-09-05  Easwaran Raman  <eraman@google.com>

    Backport from mainline
        PR rtl-optimization/62146
        * ifcvt.c (dead_or_predicable): Make removal of REG_EQUAL note of
            hoisted instruction unconditional.

    testsuite:
        * testsuite/g++.dg/opt/pr62146.C: New.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/g++.dg/opt/pr62146.C
      - copied unchanged from r214977, trunk/gcc/testsuite/g++.dg/opt/pr62146.C
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/ifcvt.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog

[Bug rtl-optimization/62146] CSE replaces constant with an expression incorrectly

[eraman at google dot com]

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

Easwaran Raman <eraman at google dot com> changed:

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

--- Comment #4 from Easwaran Raman <eraman at google dot com> ---
Google ref: b/16870586