[Bug c/110634] New: Incorrect RISC-V assembly with -fno-omit-frame-pointer

[Bug c/110634] New: Incorrect RISC-V assembly with -fno-omit-frame-pointer

[bjorn at kernel dot org]

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

            Bug ID: 110634
           Summary: Incorrect RISC-V assembly with -fno-omit-frame-pointer
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bjorn at kernel dot org
  Target Milestone: ---

Is see inconsistencies when comparing -fno-omit-frame-pointer GCC and clang
builds. 

$ cat foo.c
int foo(int a, int b) {
  return a + b;
}

$ clang-17 --target=riscv64-linux-gnu -O3 -fno-omit-frame-pointer -c foo.c -o
foo.o && riscv64-linux-gnu-objdump -d foo.o 

foo.o:     file format elf64-littleriscv


Disassembly of section .text:

0000000000000000 <foo>:
   0:   1141                    add     sp,sp,-16
   2:   e406                    sd      ra,8(sp)
   4:   e022                    sd      s0,0(sp)
   6:   0800                    add     s0,sp,16
   8:   9d2d                    addw    a0,a0,a1
   a:   60a2                    ld      ra,8(sp)
   c:   6402                    ld      s0,0(sp)
   e:   0141                    add     sp,sp,16
  10:   8082                    ret


Where GCC yields:

$ riscv64-linux-gnu-gcc -O3 -fno-omit-frame-pointer -c foo.c -o foo.o &&
riscv64-linux-gnu-objdump -d foo.o 

foo.o:     file format elf64-littleriscv


Disassembly of section .text:

0000000000000000 <foo>:
   0:   1141                    add     sp,sp,-16
   2:   e422                    sd      s0,8(sp)
   4:   0800                    add     s0,sp,16
   6:   6422                    ld      s0,8(sp)
   8:   9d2d                    addw    a0,a0,a1
   a:   0141                    add     sp,sp,16
   c:   8082                    ret


I would expect GCC to store "ra" similar to clang, but the spec is a bit vague
[1].

https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#frame-pointer-convention

[Bug target/110634] Incorrect RISC-V assembly with -fno-omit-frame-pointer

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2023-07-11

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't see where in any of the spec mentioned that storing of ra is needed at
all. That is it does not read ambigous to me at all. It just mentions for a
frame pointer, the frame pointer needs to be saved and nothing about ra.

This is totally different from the power ABI.

[Bug target/110634] Incorrect RISC-V assembly with -fno-omit-frame-pointer

[bjorn at kernel dot org]

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

--- Comment #2 from Björn Töpel <bjorn at kernel dot org> ---
(In reply to Andrew Pinski from comment #1)
> I don't see where in any of the spec mentioned that storing of ra is needed
> at all. That is it does not read ambigous to me at all. It just mentions for
> a frame pointer, the frame pointer needs to be saved and nothing about ra.
> 
> This is totally different from the power ABI.

Hmm, but is a frame-pointer w/o the ability to unwind useful? Or am I missing
something?

[Bug target/110634] Incorrect RISC-V assembly with -fno-omit-frame-pointer

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Björn Töpel from comment #2)
> Hmm, but is a frame-pointer w/o the ability to unwind useful? Or am I
> missing something?

You know what, you are better ask the ABI folks because GCC follows the ABI
specifically.

[Bug target/110634] Incorrect RISC-V assembly with -fno-omit-frame-pointer

[schwab@linux-m68k.org]

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

--- Comment #4 from Andreas Schwab <schwab@linux-m68k.org> ---
You need to look at a non-leaf function.  For a leaf function there is nothing
to unwind from.

[Bug target/110634] Incorrect RISC-V assembly with -fno-omit-frame-pointer

[cleger at rivosinc dot com]

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

Clément Léger <cleger at rivosinc dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cleger at rivosinc dot com

--- Comment #5 from Clément Léger <cleger at rivosinc dot com> ---
(In reply to Andrew Pinski from comment #1)
> I don't see where in any of the spec mentioned that storing of ra is needed
> at all. That is it does not read ambigous to me at all. It just mentions for
> a frame pointer, the frame pointer needs to be saved and nothing about ra.
> 
> This is totally different from the power ABI.

I also stumbled on this problem and looking at the spec, it seems specified
what needs to be stored in a frame record:

"A frame record consists of two XLEN values on the stack; the return address
and the link to the next frame record. The frame pointer register will point to
the innermost frame, thereby starting the linked list. By convention, the
lowest XLEN value shall point to the previous frame, while the next XLEN value
shall be the return address."

So storing ra is actually specified unless you refer to an older version of
this spec but this clarification was added in a (somehow) not so recent commit:
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/e353f995d0645078e4ce5cb1acd355d37cb3e9c2

Regarding frame pointer generation wrt to leaf functions, either GCC should not
generate the frame record or generate a non bogus one. The current one is
unusable since it is malformed. But this behavior should probably depends on
-f(no)omit-frame-leaf-pointer.