[Bug c++/113023] New: RISCV redundant code for loading fixed address

[Bug c++/113023] New: RISCV redundant code for loading fixed address

[iwfinlay at gmail dot com]

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

            Bug ID: 113023
           Summary: RISCV redundant code for loading fixed address
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: iwfinlay at gmail dot com
  Target Milestone: ---

Created attachment 56879
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56879&action=edit
Save temps from: riscv64-unknown-elf-g++ --save-temps -march=rv32imv_zbb_zbs
-mabi=ilp32 -O3 -Wall -Wextra -c test.cpp

Load immediate of a constant leaves a redundant addi x, x, 0 (aka mv x, x) for
the lower bits of the constant in some cases. The generated code is
functionally correct but loses performance.

void pcnt0() updates a global structure. int pcnt1() returns the value. Both
access the global structure for the input but pcnt0() has the redundant mv a5,
a5. Note that the use of cpop is not significant (other functions show the same
behavior in the attached test case).

Disassembly of section .text:

00000028 <_Z5pcnt0v>:
  28:   000007b7                lui     a5,0x0
  2c:   00078793                mv      a5,a5
  30:   0047a703                lw      a4,4(a5) # 4 <_Z6globalv+0x4>
  34:   60271713                cpop    a4,a4
  38:   00e7a023                sw      a4,0(a5)
  3c:   00008067                ret

00000040 <_Z5pcnt1v>:
  40:   000007b7                lui     a5,0x0
  44:   0047a503                lw      a0,4(a5) # 4 <_Z6globalv+0x4>
  48:   60251513                cpop    a0,a0
  4c:   00008067                ret

riscv64-unknown-elf-g++ --save-temps -march=rv32imv_zbb_zbs -mabi=ilp32 -O3
-Wall -Wextra -c test.cpp
riscv64-unknown-elf-objdump -d test.o > test.asm (subset shown above)

A brew install of GCC for RISCV is used for this case. The problem originated
in client code with a customized compiler build (also 12.2).

[Bug target/113023] RISCV redundant code for loading fixed address

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
_Z5pcnt1v:

        lui     a5,%hi(.LANCHOR0)
        addi    a5,a5,%lo(.LANCHOR0)
        lw      a4,4(a5)
#APP
# 16 "test.cpp" 1
        cpop a4, a4
# 0 "" 2
#NO_APP
        sw      a4,0(a5)


...


_Z5pcnt0v:

        lui     a5,%hi(.LANCHOR0+4)
        lw      a0,%lo(.LANCHOR0+4)(a5)


From GCC point of view this is correct as GCC does not know if %hi(.LANCHOR0+4)
== %hi(.LANCHOR0)

[Bug target/113023] RISCV redundant code for loading fixed address

[schwab@linux-m68k.org]

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

--- Comment #2 from Andreas Schwab <schwab@linux-m68k.org> ---
The insn is _not_ redundant, there is a relocation on it.  The linker
relaxation will eventually remove it when it becomes unnessessary.

[Bug target/113023] RISCV redundant code for loading fixed address

[iwfinlay at gmail dot com]

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

--- Comment #3 from Iain Finlay <iwfinlay at gmail dot com> ---
It does not get removed. It ends up in the final image. It is also redundant
because load and store can also add a 12 bit signed offset.

[Bug target/113023] RISCV redundant code for loading fixed address

[iwfinlay at gmail dot com]

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

--- Comment #4 from Iain Finlay <iwfinlay at gmail dot com> ---
GCC does know that it needs LANCHOR0 and LANCHOR0+4 (meaning a difference of
4). The 12-bit lower portion can be provided in the load and store commands. It
seems just an implementation choice in pcnt0 that it commits to the addi rather
than use the lw/sw, no?

[Bug target/113023] RISCV redundant code for loading fixed address

[schwab@linux-m68k.org]

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

--- Comment #5 from Andreas Schwab <schwab@linux-m68k.org> ---
If the linker relaxation does not remove a relaxable move then it is a bug in
the linker.