[Bug target/97682] New: Miscompiled tail call with -fPIC

[Bug target/97682] New: Miscompiled tail call with -fPIC

[schwab@linux-m68k.org]

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

            Bug ID: 97682
           Summary: Miscompiled tail call with -fPIC
           Product: gcc
           Version: 10.2.1
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: schwab@linux-m68k.org
  Target Milestone: ---
            Target: riscv64-*-*

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

When the attached file is compiled with -fPIC -O3, the tail call to
std::vector<llvm::BlockFrequencyInfoImplBase::FrequencyData,
std::allocator<llvm::BlockFrequencyInfoImplBase::FrequencyData>
>::_M_default_append(unsigned long) is missing in
llvm::BlockFrequencyInfoImpl<llvm::BasicBlock>::initializeRPOT():

.L2924:
        li      t1,4096         #,
        add     sp,sp,t1        #,,
        ld      ra,376(sp)              #,
        ld      s0,368(sp)              #,
        ld      s1,360(sp)              #,
        ld      s3,344(sp)              #,
        ld      s4,336(sp)              #,
        ld      s5,328(sp)              #,
        ld      s6,320(sp)              #,
        ld      s7,312(sp)              #,
        ld      s8,304(sp)              #,
        ld      s9,296(sp)              #,
        ld      s10,288(sp)             #,
        ld      s11,280(sp)             #,
# /usr/include/c++/10/bits/stl_vector.h:940:      _M_default_append(__new_size
- size());
        addi    a0,s2,8 #,, this
        ld      s2,352(sp)              #,
# /usr/include/c++/10/bits/stl_vector.h:940:      _M_default_append(__new_size
- size());
        sub     a1,a5,a1        #, _680, tmp1548
        addi    sp,sp,384       #,,
# /usr/include/c++/10/bits/stl_vector.h:940:      _M_default_append(__new_size
- size());
        jr      t1              #

Without -fPIC, the last line has the correct tail call:

        tail   
_ZNSt6vectorIN4llvm26BlockFrequencyInfoImplBase13FrequencyDataESaIS2_EE17_M_default_appendEm
           #

[Bug target/97682] Miscompiled tail call with -fPIC

[schwab@linux-m68k.org]

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

--- Comment #1 from Andreas Schwab <schwab@linux-m68k.org> ---
Looks like the miscompilation happens in the pro_and_epilog pass.

Before:

(insn 2520 2519 2521 320 (set (reg:DI 6 t1)
        (symbol_ref/i:DI
("_ZNSt6vectorIN4llvm26BlockFrequencyInfoImplBase13FrequencyDataESaIS2_EE17_M_default_appendEm")
[flags 0x1]  <function_decl 0x7fa72f945900 _M_default_append>))
"/usr/include/c++/10/bits/stl_vector.h":940:21 135 {*movdi_64bit}
     (nil))
(call_insn/j 2521 2520 2522 320 (call (mem:SI (reg:DI 6 t1) [0
_M_default_append S4 A32])
        (const_int 0 [0])) "/usr/include/c++/10/bits/stl_vector.h":940:21 251
{sibcall_internal}
     (expr_list:REG_CALL_DECL (symbol_ref/i:DI
("_ZNSt6vectorIN4llvm26BlockFrequencyInfoImplBase13FrequencyDataESaIS2_EE17_M_default_appendEm")
[flags 0x1]  <function_decl 0x7fa72f945900 _M_default_append>)
        (expr_list:REG_EH_REGION (const_int 0 [0])
            (nil)))
    (expr_list:DI (use (reg:DI 10 a0))
        (expr_list:DI (use (reg:DI 11 a1))
            (nil))))
(barrier 2522 2521 2523)

After:

(insn 2520 2519 5286 317 (set (reg:DI 6 t1)
        (symbol_ref/i:DI
("_ZNSt6vectorIN4llvm26BlockFrequencyInfoImplBase13FrequencyDataESaIS2_EE17_M_default_appendEm")
[flags 0x1]  <function_decl 0x7fa72f945900 _M_default_append>))
"/usr/include/c++/10/bits/stl_vector.h":940:21 135 {*movdi_64bit}
     (nil))
(note 5286 2520 5269 317 NOTE_INSN_EPILOGUE_BEG)
(insn 5269 5286 5270 317 (set (mem:BLK (scratch) [0  A8])
        (unspec:BLK [
                (reg/f:DI 2 sp)
                (reg/f:DI 8 s0)
            ] UNSPEC_TIE))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (nil))
(insn 5270 5269 5271 317 (set (reg:DI 6 t1)
        (const_int 4096 [0x1000]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (nil))
(insn/f 5271 5270 5272 317 (set (reg/f:DI 2 sp)
        (plus:DI (reg/f:DI 2 sp)
            (reg:DI 6 t1)))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_DEF_CFA (plus:DI (reg/f:DI 2 sp)
            (const_int 384 [0x180]))
        (nil)))
(insn/f 5272 5271 5273 317 (set (reg:DI 1 ra)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 376 [0x178])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 1 ra)
        (nil)))
(insn/f 5273 5272 5274 317 (set (reg:DI 8 s0)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 368 [0x170])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 8 s0)
        (nil)))
(insn/f 5274 5273 5275 317 (set (reg:DI 9 s1)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 360 [0x168])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 9 s1)
        (nil)))
(insn/f 5275 5274 5276 317 (set (reg:DI 18 s2)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 352 [0x160])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 18 s2)
        (nil)))
(insn/f 5276 5275 5277 317 (set (reg:DI 19 s3)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 344 [0x158])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 19 s3)
        (nil)))
(insn/f 5277 5276 5278 317 (set (reg:DI 20 s4)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 336 [0x150])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 20 s4)
        (nil)))
(insn/f 5278 5277 5279 317 (set (reg:DI 21 s5)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 328 [0x148])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 21 s5)
        (nil)))
(insn/f 5279 5278 5280 317 (set (reg:DI 22 s6)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 320 [0x140])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 22 s6)
        (nil)))
(insn/f 5280 5279 5281 317 (set (reg:DI 23 s7)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 312 [0x138])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 23 s7)
        (nil)))
(insn/f 5281 5280 5282 317 (set (reg:DI 24 s8)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 304 [0x130])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 24 s8)
        (nil)))
(insn/f 5282 5281 5283 317 (set (reg:DI 25 s9)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 296 [0x128])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 25 s9)
        (nil)))
(insn/f 5283 5282 5284 317 (set (reg:DI 26 s10)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 288 [0x120])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 26 s10)
        (nil)))
(insn/f 5284 5283 5285 317 (set (reg:DI 27 s11)
        (mem/c:DI (plus:DI (reg/f:DI 2 sp)
                (const_int 280 [0x118])) [537  S8 A64]))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 27 s11)
        (nil)))
(insn/f 5285 5284 2521 317 (set (reg/f:DI 2 sp)
        (plus:DI (reg/f:DI 2 sp)
            (const_int 384 [0x180])))
"../include/llvm/Analysis/BlockFrequencyInfoImpl.h":1145:1 -1
     (expr_list:REG_CFA_DEF_CFA (plus:DI (reg/f:DI 2 sp)
            (const_int 0 [0]))
        (nil)))
(call_insn/j 2521 5285 2522 317 (call (mem:SI (reg:DI 6 t1) [0
_M_default_append S4 A32])
        (const_int 0 [0])) "/usr/include/c++/10/bits/stl_vector.h":940:21 251
{sibcall_internal}
     (expr_list:REG_CALL_DECL (symbol_ref/i:DI
("_ZNSt6vectorIN4llvm26BlockFrequencyInfoImplBase13FrequencyDataESaIS2_EE17_M_default_appendEm")
[flags 0x1]  <function_decl 0x7fa72f945900 _M_default_append>)
        (expr_list:REG_EH_REGION (const_int 0 [0])
            (nil)))
    (expr_list:DI (use (reg:DI 10 a0))
        (expr_list:DI (use (reg:DI 11 a1))
            (nil))))

[Bug target/97682] Miscompiled tail call with -fPIC

[schwab@linux-m68k.org]

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

--- Comment #2 from Andreas Schwab <schwab@linux-m68k.org> ---
I think the bug is really that riscv_legitimize_call_address uses
RISCV_PROLOGUE_TEMP, which can conflict with its uses in the epilogue, as seen.

[Bug target/97682] Miscompiled tail call with -fPIC

[kito at gcc dot gnu.org]

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

Kito Cheng <kito at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-11-03
                 CC|                            |kito at gcc dot gnu.org

--- Comment #3 from Kito Cheng <kito at gcc dot gnu.org> ---
Confirmed, I got this issue few years ago when implementing large code model on
GCC, but I never upstream or open source that, and I have no chance to access
it now...anyway, our guys is fixing that now, thanks your report!

[Bug target/97682] Miscompiled tail call with -fPIC

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jim Wilson <wilson@gcc.gnu.org>:

https://gcc.gnu.org/g:207de83922bda8707aa33d6a2185e691116377e7

commit r11-5026-g207de83922bda8707aa33d6a2185e691116377e7
Author: Monk Chiang <monk.chiang@sifive.com>
Date:   Fri Nov 13 19:35:11 2020 -0800

    PR target/97682 - Fix to reuse t1 register between call address and
epilogue.

      - When expanding the call pattern, choose t1 register be a jump register.
        Epilogue also uses a t1 register to adjust Stack point. The call
pattern
        and epilogue will initial t1 twice, if both are generated in the same
        function. The call pattern will emit 'la t1,symbol' and 'jalr
t1'instructions.
        Epilogue also emits 'li t1,4096' and 'addi sp,sp,t1' instructions.
        But li and addi instructions will be placed between la and jalr
instructions.
        The la instruction will be removed by some optimizations,
        because t1 register define twice, the first define instruction look
        likes duplicate.

      - To resolve this issue, Prologue and Epilogue use the t0 register
        be a temporary register, the call pattern use the t1 register be
        a temporary register.

    gcc/
    2020-11-13  Monk Chiang  <monk.chiang@sifive.com>

            PR target/97682
            * config/riscv/riscv.h (RISCV_PROLOGUE_TEMP_REGNUM): Change
register
            to t0.
            (RISCV_CALL_ADDRESS_TEMP_REGNUM): New Marco, define t1 register.
            (RISCV_CALL_ADDRESS_TEMP): Use it for call instructions.
            * config/riscv/riscv.c (riscv_legitimize_call_address): Use
            RISCV_CALL_ADDRESS_TEMP.
            (riscv_compute_frame_info): Change temporary register to t0 form
t1.
            (riscv_trampoline_init): Adjust comment.

    gcc/testsuite/
    2020-11-13  Monk Chiang  <monk.chiang@sifive.com>

            PR target/97682
            * g++.target/riscv/pr97682.C: New test.
            * gcc.target/riscv/interrupt-3.c: Check register for t0.
            * gcc.target/riscv/interrupt-4.c: Likewise.

[Bug target/97682] Miscompiled tail call with -fPIC

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Kito Cheng <kito@gcc.gnu.org>:

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

commit r10-9033-g4ba2f918583089172ac899c8eecaddef0d47cd85
Author: Monk Chiang <monk.chiang@sifive.com>
Date:   Fri Nov 13 19:35:11 2020 -0800

    PR target/97682 - Fix to reuse t1 register between call address and
epilogue.

      - When expanding the call pattern, choose t1 register be a jump register.
        Epilogue also uses a t1 register to adjust Stack point. The call
pattern
        and epilogue will initial t1 twice, if both are generated in the same
        function. The call pattern will emit 'la t1,symbol' and 'jalr
t1'instructions.
        Epilogue also emits 'li t1,4096' and 'addi sp,sp,t1' instructions.
        But li and addi instructions will be placed between la and jalr
instructions.
        The la instruction will be removed by some optimizations,
        because t1 register define twice, the first define instruction look
        likes duplicate.

      - To resolve this issue, Prologue and Epilogue use the t0 register
        be a temporary register, the call pattern use the t1 register be
        a temporary register.

    gcc/
    2020-11-13  Monk Chiang  <monk.chiang@sifive.com>

            PR target/97682
            * config/riscv/riscv.h (RISCV_PROLOGUE_TEMP_REGNUM): Change
register
            to t0.
            (RISCV_CALL_ADDRESS_TEMP_REGNUM): New Marco, define t1 register.
            (RISCV_CALL_ADDRESS_TEMP): Use it for call instructions.
            * config/riscv/riscv.c (riscv_legitimize_call_address): Use
            RISCV_CALL_ADDRESS_TEMP.
            (riscv_compute_frame_info): Change temporary register to t0 form
t1.
            (riscv_trampoline_init): Adjust comment.

    gcc/testsuite/
    2020-11-13  Monk Chiang  <monk.chiang@sifive.com>

            PR target/97682
            * g++.target/riscv/pr97682.C: New test.
            * gcc.target/riscv/interrupt-3.c: Check register for t0.
            * gcc.target/riscv/interrupt-4.c: Likewise.

    (cherry picked from commit 207de83922bda8707aa33d6a2185e691116377e7)

[Bug target/97682] Miscompiled tail call with -fPIC

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Kito Cheng <kito@gcc.gnu.org>:

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

commit r9-9051-gc52868904b784f5a90db6cf347edba81d14cf921
Author: Monk Chiang <monk.chiang@sifive.com>
Date:   Fri Nov 13 19:35:11 2020 -0800

    PR target/97682 - Fix to reuse t1 register between call address and
epilogue.

      - When expanding the call pattern, choose t1 register be a jump register.
        Epilogue also uses a t1 register to adjust Stack point. The call
pattern
        and epilogue will initial t1 twice, if both are generated in the same
        function. The call pattern will emit 'la t1,symbol' and 'jalr
t1'instructions.
        Epilogue also emits 'li t1,4096' and 'addi sp,sp,t1' instructions.
        But li and addi instructions will be placed between la and jalr
instructions.
        The la instruction will be removed by some optimizations,
        because t1 register define twice, the first define instruction look
        likes duplicate.

      - To resolve this issue, Prologue and Epilogue use the t0 register
        be a temporary register, the call pattern use the t1 register be
        a temporary register.

    gcc/
    2020-11-13  Monk Chiang  <monk.chiang@sifive.com>

            PR target/97682
            * config/riscv/riscv.h (RISCV_PROLOGUE_TEMP_REGNUM): Change
register
            to t0.
            (RISCV_CALL_ADDRESS_TEMP_REGNUM): New Marco, define t1 register.
            (RISCV_CALL_ADDRESS_TEMP): Use it for call instructions.
            * config/riscv/riscv.c (riscv_legitimize_call_address): Use
            RISCV_CALL_ADDRESS_TEMP.
            (riscv_compute_frame_info): Change temporary register to t0 form
t1.
            (riscv_trampoline_init): Adjust comment.

    gcc/testsuite/
    2020-11-13  Monk Chiang  <monk.chiang@sifive.com>

            PR target/97682
            * g++.target/riscv/pr97682.C: New test.
            * gcc.target/riscv/interrupt-3.c: Check register for t0.
            * gcc.target/riscv/interrupt-4.c: Likewise.

    (cherry picked from commit 207de83922bda8707aa33d6a2185e691116377e7)

[Bug target/97682] Miscompiled tail call with -fPIC

[kito at gcc dot gnu.org]

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

Kito Cheng <kito at gcc dot gnu.org> changed:

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

--- Comment #7 from Kito Cheng <kito at gcc dot gnu.org> ---
Fixed on trunk and backport to gcc 9 and gcc 10.