[RISC-V] epilogue_begin is set incorrectly #120553

RamNalamothu · 2024-12-19T11:33:09Z

For the following test,

int main(int argc, char **argv)
{
  int foo = 1;

  return 0;
}

GCC (riscv32) generates (https://godbolt.org/z/Wqze6n8Gs):

main:
.LFB0:
        .file 1 "/app/example.c"
        .loc 1 2 1
        .cfi_startproc
        addi    sp,sp,-48
        .cfi_def_cfa_offset 48
        sw      ra,44(sp)
        sw      s0,40(sp)
        .cfi_offset 1, -4
        .cfi_offset 8, -8
        addi    s0,sp,48
        .cfi_def_cfa 8, 0
        sw      a0,-36(s0)
        sw      a1,-40(s0)
        .loc 1 3 7
        li      a5,1
        sw      a5,-20(s0)
        .loc 1 5 10
        li      a5,0
        .loc 1 6 1
        mv      a0,a5
        lw      ra,44(sp)
        .cfi_restore 1
        lw      s0,40(sp)
        .cfi_restore 8
        .cfi_def_cfa 2, 48
        addi    sp,sp,48
        .cfi_def_cfa_offset 0
        jr      ra
        .cfi_endproc
.LFE0:
        .size   main, .-main

and Clang riscv32 generates (https://godbolt.org/z/TMGPsr44s):

main:
.Lfunc_begin0:
        .file   0 "/app" "/app/example.c" md5 0x8aa04db627be45b3215ab92eac2e23c5
        .file   1 "example.c" md5 0x8aa04db627be45b3215ab92eac2e23c5
        .loc    1 2 0
        .cfi_sections .debug_frame
        .cfi_startproc
        addi    sp, sp, -32
        .cfi_def_cfa_offset 32
        sw      ra, 28(sp)
        sw      s0, 24(sp)
        .cfi_offset ra, -4
        .cfi_offset s0, -8
        addi    s0, sp, 32
        .cfi_def_cfa s0, 0
        mv      a2, a0
        li      a0, 0
        sw      a0, -12(s0)
        sw      a2, -16(s0)
        sw      a1, -20(s0)
        li      a1, 1
.Ltmp0:
        .loc    1 3 7 prologue_end
        sw      a1, -24(s0)
        .cfi_def_cfa sp, 32
        lw      ra, 28(sp)                                        <<<<<<<<<<
        lw      s0, 24(sp)
        .cfi_restore ra
        .cfi_restore s0
        .loc    1 5 3 epilogue_begin                               <<<<<<<<<<
        addi    sp, sp, 32
        .cfi_def_cfa_offset 0
        ret
.Ltmp1:
.Lfunc_end0:
        .size   main, .Lfunc_end0-main

As can be seen from the Clang's output, the epilogue_begin is set after the epilogue has actually begun. This creates a problem if we single step from line 3, or set a breakpoint on line 5, the FP has been restored to the parent frame's FP, and accessing variables goes to the wrong place.

$ gdb main.gcc

(gdb) b main
Breakpoint 1 at 0x10444: file main.c, line 3.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1, argv=0x40800004) at main.c:3
3                 int foo = 1;
(gdb) si
0x00010446      3                 int foo = 1;
(gdb) p foo
$1 = 0
(gdb) si
5                   return 0;
(gdb) p foo
$2 = 1
(gdb) s
6       }
(gdb) p foo
$3 = 1
(gdb)

$ gdb main.llvm

(gdb) b main
Breakpoint 1 at 0x10496: file main.c, line 3.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1, argv=0x407ffff4) at main.c:3
3                 int foo = 1;
(gdb) si
0x0001049a      3                 int foo = 1;
(gdb) p foo
$1 = 1
(gdb) si
0x0001049c      3                 int foo = 1;
(gdb) p foo
$2 = 1
(gdb) s
5                   return 0;
(gdb) p foo
$3 = -1242739775
(gdb)

The text was updated successfully, but these errors were encountered:

llvmbot · 2024-12-19T11:33:29Z

@llvm/issue-subscribers-bug

Author: Venkata Ramanaiah Nalamothu (RamNalamothu)

For the following test, ``` int main(int argc, char **argv) { int foo = 1;

return 0;
}

GCC (riscv32) generates (https://godbolt.org/z/Wqze6n8Gs):

main:
.LFB0:
.file 1 "/app/example.c"
.loc 1 2 1
.cfi_startproc
addi sp,sp,-48
.cfi_def_cfa_offset 48
sw ra,44(sp)
sw s0,40(sp)
.cfi_offset 1, -4
.cfi_offset 8, -8
addi s0,sp,48
.cfi_def_cfa 8, 0
sw a0,-36(s0)
sw a1,-40(s0)
.loc 1 3 7
li a5,1
sw a5,-20(s0)
.loc 1 5 10
li a5,0
.loc 1 6 1
mv a0,a5
lw ra,44(sp)
.cfi_restore 1
lw s0,40(sp)
.cfi_restore 8
.cfi_def_cfa 2, 48
addi sp,sp,48
.cfi_def_cfa_offset 0
jr ra
.cfi_endproc
.LFE0:
.size main, .-main

and Clang riscv32 generates (https://godbolt.org/z/TMGPsr44s):

main:
.Lfunc_begin0:
.file 0 "/app" "/app/example.c" md5 0x8aa04db627be45b3215ab92eac2e23c5
.file 1 "example.c" md5 0x8aa04db627be45b3215ab92eac2e23c5
.loc 1 2 0
.cfi_sections .debug_frame
.cfi_startproc
addi sp, sp, -32
.cfi_def_cfa_offset 32
sw ra, 28(sp)
sw s0, 24(sp)
.cfi_offset ra, -4
.cfi_offset s0, -8
addi s0, sp, 32
.cfi_def_cfa s0, 0
mv a2, a0
li a0, 0
sw a0, -12(s0)
sw a2, -16(s0)
sw a1, -20(s0)
li a1, 1
.Ltmp0:
.loc 1 3 7 prologue_end
sw a1, -24(s0)
.cfi_def_cfa sp, 32
lw ra, 28(sp) <<<<<<<<<<
lw s0, 24(sp)
.cfi_restore ra
.cfi_restore s0
.loc 1 5 3 epilogue_begin <<<<<<<<<<
addi sp, sp, 32
.cfi_def_cfa_offset 0
ret
.Ltmp1:
.Lfunc_end0:
.size main, .Lfunc_end0-main

As can be seen from the Clang's output, the _epilogue_begin_ is set after the epilogue has actually begun. This creates a problem if we single step from line 3, or set a breakpoint on line 5, the FP has been restored to the parent frame's FP, and accessing variables goes to the wrong place.

$ gdb main.gcc

(gdb) b main
Breakpoint 1 at 0x10444: file main.c, line 3.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1, argv=0x40800004) at main.c:3
3 int foo = 1;
(gdb) si
0x00010446 3 int foo = 1;
(gdb) p foo
$1 = 0
(gdb) si
5 return 0;
(gdb) p foo
$2 = 1
(gdb) s
6 }
(gdb) p foo
$3 = 1
(gdb)

$ gdb main.llvm

(gdb) b main
Breakpoint 1 at 0x10496: file main.c, line 3.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1, argv=0x407ffff4) at main.c:3
3 int foo = 1;
(gdb) si
0x0001049a 3 int foo = 1;
(gdb) p foo
$1 = 1
(gdb) si
0x0001049c 3 int foo = 1;
(gdb) p foo
$2 = 1
(gdb) s
5 return 0;
(gdb) p foo
$3 = -1242739775
(gdb)

</details>

llvmbot · 2024-12-19T11:33:29Z

@llvm/issue-subscribers-debuginfo

Author: Venkata Ramanaiah Nalamothu (RamNalamothu)

For the following test, ``` int main(int argc, char **argv) { int foo = 1;

return 0;
}

GCC (riscv32) generates (https://godbolt.org/z/Wqze6n8Gs):

main:
.LFB0:
.file 1 "/app/example.c"
.loc 1 2 1
.cfi_startproc
addi sp,sp,-48
.cfi_def_cfa_offset 48
sw ra,44(sp)
sw s0,40(sp)
.cfi_offset 1, -4
.cfi_offset 8, -8
addi s0,sp,48
.cfi_def_cfa 8, 0
sw a0,-36(s0)
sw a1,-40(s0)
.loc 1 3 7
li a5,1
sw a5,-20(s0)
.loc 1 5 10
li a5,0
.loc 1 6 1
mv a0,a5
lw ra,44(sp)
.cfi_restore 1
lw s0,40(sp)
.cfi_restore 8
.cfi_def_cfa 2, 48
addi sp,sp,48
.cfi_def_cfa_offset 0
jr ra
.cfi_endproc
.LFE0:
.size main, .-main

and Clang riscv32 generates (https://godbolt.org/z/TMGPsr44s):

main:
.Lfunc_begin0:
.file 0 "/app" "/app/example.c" md5 0x8aa04db627be45b3215ab92eac2e23c5
.file 1 "example.c" md5 0x8aa04db627be45b3215ab92eac2e23c5
.loc 1 2 0
.cfi_sections .debug_frame
.cfi_startproc
addi sp, sp, -32
.cfi_def_cfa_offset 32
sw ra, 28(sp)
sw s0, 24(sp)
.cfi_offset ra, -4
.cfi_offset s0, -8
addi s0, sp, 32
.cfi_def_cfa s0, 0
mv a2, a0
li a0, 0
sw a0, -12(s0)
sw a2, -16(s0)
sw a1, -20(s0)
li a1, 1
.Ltmp0:
.loc 1 3 7 prologue_end
sw a1, -24(s0)
.cfi_def_cfa sp, 32
lw ra, 28(sp) <<<<<<<<<<
lw s0, 24(sp)
.cfi_restore ra
.cfi_restore s0
.loc 1 5 3 epilogue_begin <<<<<<<<<<
addi sp, sp, 32
.cfi_def_cfa_offset 0
ret
.Ltmp1:
.Lfunc_end0:
.size main, .Lfunc_end0-main

As can be seen from the Clang's output, the _epilogue_begin_ is set after the epilogue has actually begun. This creates a problem if we single step from line 3, or set a breakpoint on line 5, the FP has been restored to the parent frame's FP, and accessing variables goes to the wrong place.

$ gdb main.gcc

(gdb) b main
Breakpoint 1 at 0x10444: file main.c, line 3.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1, argv=0x40800004) at main.c:3
3 int foo = 1;
(gdb) si
0x00010446 3 int foo = 1;
(gdb) p foo
$1 = 0
(gdb) si
5 return 0;
(gdb) p foo
$2 = 1
(gdb) s
6 }
(gdb) p foo
$3 = 1
(gdb)

$ gdb main.llvm

(gdb) b main
Breakpoint 1 at 0x10496: file main.c, line 3.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1, argv=0x407ffff4) at main.c:3
3 int foo = 1;
(gdb) si
0x0001049a 3 int foo = 1;
(gdb) p foo
$1 = 1
(gdb) si
0x0001049c 3 int foo = 1;
(gdb) p foo
$2 = 1
(gdb) s
5 return 0;
(gdb) p foo
$3 = -1242739775
(gdb)

</details>

llvmbot · 2024-12-19T11:33:30Z

@llvm/issue-subscribers-backend-risc-v

Author: Venkata Ramanaiah Nalamothu (RamNalamothu)

For the following test, ``` int main(int argc, char **argv) { int foo = 1;

return 0;
}

GCC (riscv32) generates (https://godbolt.org/z/Wqze6n8Gs):

main:
.LFB0:
.file 1 "/app/example.c"
.loc 1 2 1
.cfi_startproc
addi sp,sp,-48
.cfi_def_cfa_offset 48
sw ra,44(sp)
sw s0,40(sp)
.cfi_offset 1, -4
.cfi_offset 8, -8
addi s0,sp,48
.cfi_def_cfa 8, 0
sw a0,-36(s0)
sw a1,-40(s0)
.loc 1 3 7
li a5,1
sw a5,-20(s0)
.loc 1 5 10
li a5,0
.loc 1 6 1
mv a0,a5
lw ra,44(sp)
.cfi_restore 1
lw s0,40(sp)
.cfi_restore 8
.cfi_def_cfa 2, 48
addi sp,sp,48
.cfi_def_cfa_offset 0
jr ra
.cfi_endproc
.LFE0:
.size main, .-main

and Clang riscv32 generates (https://godbolt.org/z/TMGPsr44s):

main:
.Lfunc_begin0:
.file 0 "/app" "/app/example.c" md5 0x8aa04db627be45b3215ab92eac2e23c5
.file 1 "example.c" md5 0x8aa04db627be45b3215ab92eac2e23c5
.loc 1 2 0
.cfi_sections .debug_frame
.cfi_startproc
addi sp, sp, -32
.cfi_def_cfa_offset 32
sw ra, 28(sp)
sw s0, 24(sp)
.cfi_offset ra, -4
.cfi_offset s0, -8
addi s0, sp, 32
.cfi_def_cfa s0, 0
mv a2, a0
li a0, 0
sw a0, -12(s0)
sw a2, -16(s0)
sw a1, -20(s0)
li a1, 1
.Ltmp0:
.loc 1 3 7 prologue_end
sw a1, -24(s0)
.cfi_def_cfa sp, 32
lw ra, 28(sp) <<<<<<<<<<
lw s0, 24(sp)
.cfi_restore ra
.cfi_restore s0
.loc 1 5 3 epilogue_begin <<<<<<<<<<
addi sp, sp, 32
.cfi_def_cfa_offset 0
ret
.Ltmp1:
.Lfunc_end0:
.size main, .Lfunc_end0-main

As can be seen from the Clang's output, the _epilogue_begin_ is set after the epilogue has actually begun. This creates a problem if we single step from line 3, or set a breakpoint on line 5, the FP has been restored to the parent frame's FP, and accessing variables goes to the wrong place.

$ gdb main.gcc

(gdb) b main
Breakpoint 1 at 0x10444: file main.c, line 3.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1, argv=0x40800004) at main.c:3
3 int foo = 1;
(gdb) si
0x00010446 3 int foo = 1;
(gdb) p foo
$1 = 0
(gdb) si
5 return 0;
(gdb) p foo
$2 = 1
(gdb) s
6 }
(gdb) p foo
$3 = 1
(gdb)

$ gdb main.llvm

(gdb) b main
Breakpoint 1 at 0x10496: file main.c, line 3.
(gdb) c
Continuing.

Breakpoint 1, main (argc=1, argv=0x407ffff4) at main.c:3
3 int foo = 1;
(gdb) si
0x0001049a 3 int foo = 1;
(gdb) p foo
$1 = 1
(gdb) si
0x0001049c 3 int foo = 1;
(gdb) p foo
$2 = 1
(gdb) s
5 return 0;
(gdb) p foo
$3 = -1242739775
(gdb)

</details>

RamNalamothu · 2024-12-19T11:35:06Z

Also, in Clang generated code, the prologue_end is set after the user code i.e. after li a1, 1, which is happening due to https://reviews.llvm.org/D48468. But GCC generated code doesn't have this issue.

I have a work in progress patch to fix the epilogue_begin issue.

wangpc-pp · 2024-12-19T11:55:40Z

Please check if this is a duplicated work: #74702

RamNalamothu · 2024-12-19T12:20:08Z

Please check if this is a duplicated work: #74702

Thanks for pointing it out but I am taking a different approach.
Hopefully, I should be able to publish the patches in the next 1-2 days.

…kSlot (NFC) This patch is in preparation to enable setting the MachineInstr::MIFlag flags, i.e. FrameSetup/FrameDestroy, on callee saved register spill/reload instructions in prologue/epilogue. This eventually helps in setting the prologue_end and epilogue_begin markers more accurately. The DWARF Spec in "6.4 Call Frame Information" says: The code that allocates space on the call frame stack and performs the save operation is called the subroutine’s prologue, and the code that performs the restore operation and deallocates the frame is called its epilogue. which means the callee saved register spills and reloads are part of prologue (a.k.a frame setup) and epilogue (a.k.a frame destruction), respectively. And, IIUC, LLVM backend uses FrameSetup/FrameDestroy flags to identify instructions that are part of call frame setup and destruction. In the trunk, while most targets consistently set FrameSetup/FrameDestroy on save/restore call frame information (CFI) instructions of callee saved registers, they do not consistently set those flags on the actual callee saved register spill/reload instructions. I believe this patch provides a clean mechanism to set FrameSetup/FrameDestroy flags on the actual callee saved register spill/reload instructions as needed. And, by having default argument of MachineInstr::NoFlags for Flags, this patch is a NFC. With this patch, the targets have to just pass FrameSetup/FrameDestroy flag to the storeRegToStackSlot/loadRegFromStackSlot calls from the target derived spillCalleeSavedRegisters and restoreCalleeSavedRegisters to set those flags on callee saved register spill/reload instructions. Also, this patch makes it very easy to set the source line information on callee saved register spill/reload instructions which is needed by the DwarfDebug.cpp implementation to set prologue_end and epilogue_begin markers more accurately. As per DwarfDebug.cpp implementation: prologue_end is the first known non-DBG_VALUE and non-FrameSetup location that marks the beginning of the function body epilogue_begin is the first FrameDestroy location that has been seen in the epilogue basic block With this patch, the targets have to just do the following to set the source line information on callee saved register spill/reload instructions, without hampering the LLVM's efforts to avoid adding source line information on the artificial code generated by the compiler. <Foo>InstrInfo::storeRegToStackSlot() { ... DebugLoc DL = Flags & MachineInstr::FrameSetup ? DebugLoc() : MBB.findDebugLoc(I); ... } <Foo>InstrInfo::loadRegFromStackSlot() { ... DebugLoc DL = Flags & MachineInstr::FrameDestroy ? MBB.findDebugLoc(I) : DebugLoc(); ... } While I understand this patch would break out-of-tree backend builds, I think it is in the right direction. One immediate use case that can benefit from this patch is fixing llvm#120553 becomes simpler.

The DwarfDebug.cpp implementation expects the epilogue instructions to have source location of last non-debug instruction after which the epilogue instructions are inserted. The epilogue_begin is set on location of the first FrameDestroy instruction with source line information that has been seen in the epilogue basic block. In the trunk, the risc-v backend sets the epilogue_begin after the epilogue has actually begun i.e. after callee saved register reloads and the source line information is not set on those reload instructions. This is leading to llvm#120553 where, while debugging, breaking on or single stepping to the epilogue_begin location will make accessing the variables from wrong place as the FP has been restored to the parent frame's FP. To fix that, this patch sets FrameSetup/FrameDestroy flags on the callee saved register spill/reload instructions which is actually correct. Then the RISCVInstrInfo::loadRegFromStackSlot uses FrameDestroy flag to identify a reload of the callee saved register in the epilogue and copies the source line information from insert position instruction to that reload instruction. Requires cce55a1 Fixes llvm#120553

…kSlot (NFC) This patch is in preparation to enable setting the MachineInstr::MIFlag flags, i.e. FrameSetup/FrameDestroy, on callee saved register spill/reload instructions in prologue/epilogue. This eventually helps in setting the prologue_end and epilogue_begin markers more accurately. The DWARF Spec in "6.4 Call Frame Information" says: The code that allocates space on the call frame stack and performs the save operation is called the subroutine’s prologue, and the code that performs the restore operation and deallocates the frame is called its epilogue. which means the callee saved register spills and reloads are part of prologue (a.k.a frame setup) and epilogue (a.k.a frame destruction), respectively. And, IIUC, LLVM backend uses FrameSetup/FrameDestroy flags to identify instructions that are part of call frame setup and destruction. In the trunk, while most targets consistently set FrameSetup/FrameDestroy on save/restore call frame information (CFI) instructions of callee saved registers, they do not consistently set those flags on the actual callee saved register spill/reload instructions. I believe this patch provides a clean mechanism to set FrameSetup/FrameDestroy flags on the actual callee saved register spill/reload instructions as needed. And, by having default argument of MachineInstr::NoFlags for Flags, this patch is a NFC. With this patch, the targets have to just pass FrameSetup/FrameDestroy flag to the storeRegToStackSlot/loadRegFromStackSlot calls from the target derived spillCalleeSavedRegisters and restoreCalleeSavedRegisters to set those flags on callee saved register spill/reload instructions. Also, this patch makes it very easy to set the source line information on callee saved register spill/reload instructions which is needed by the DwarfDebug.cpp implementation to set prologue_end and epilogue_begin markers more accurately. As per DwarfDebug.cpp implementation: prologue_end is the first known non-DBG_VALUE and non-FrameSetup location that marks the beginning of the function body epilogue_begin is the first FrameDestroy location that has been seen in the epilogue basic block With this patch, the targets have to just do the following to set the source line information on callee saved register spill/reload instructions, without hampering the LLVM's efforts to avoid adding source line information on the artificial code generated by the compiler. <Foo>InstrInfo::storeRegToStackSlot() { ... DebugLoc DL = Flags & MachineInstr::FrameSetup ? DebugLoc() : MBB.findDebugLoc(I); ... } <Foo>InstrInfo::loadRegFromStackSlot() { ... DebugLoc DL = Flags & MachineInstr::FrameDestroy ? MBB.findDebugLoc(I) : DebugLoc(); ... } While I understand this patch would break out-of-tree backend builds, I think it is in the right direction. One immediate use case that can benefit from this patch is fixing llvm#120553 becomes simpler.

The DwarfDebug.cpp implementation expects the epilogue instructions to have source location of last non-debug instruction after which the epilogue instructions are inserted. The epilogue_begin is set on location of the first FrameDestroy instruction with source line information that has been seen in the epilogue basic block. In the trunk, the risc-v backend sets the epilogue_begin after the epilogue has actually begun i.e. after callee saved register reloads and the source line information is not set on those reload instructions. This is leading to llvm#120553 where, while debugging, breaking on or single stepping to the epilogue_begin location will make accessing the variables from wrong place as the FP has been restored to the parent frame's FP. To fix that, this patch sets FrameSetup/FrameDestroy flags on the callee saved register spill/reload instructions which is actually correct. Then the RISCVInstrInfo::loadRegFromStackSlot uses FrameDestroy flag to identify a reload of the callee saved register in the epilogue and copies the source line information from insert position instruction to that reload instruction. Requires cce55a1 Fixes llvm#120553

…kSlot (NFC) (#120622) This patch is in preparation to enable setting the MachineInstr::MIFlag flags, i.e. FrameSetup/FrameDestroy, on callee saved register spill/reload instructions in prologue/epilogue. This eventually helps in setting the prologue_end and epilogue_begin markers more accurately. The DWARF Spec in "6.4 Call Frame Information" says: The code that allocates space on the call frame stack and performs the save operation is called the subroutine’s prologue, and the code that performs the restore operation and deallocates the frame is called its epilogue. which means the callee saved register spills and reloads are part of prologue (a.k.a frame setup) and epilogue (a.k.a frame destruction), respectively. And, IIUC, LLVM backend uses FrameSetup/FrameDestroy flags to identify instructions that are part of call frame setup and destruction. In the trunk, while most targets consistently set FrameSetup/FrameDestroy on save/restore call frame information (CFI) instructions of callee saved registers, they do not consistently set those flags on the actual callee saved register spill/reload instructions. I believe this patch provides a clean mechanism to set FrameSetup/FrameDestroy flags on the actual callee saved register spill/reload instructions as needed. And, by having default argument of MachineInstr::NoFlags for Flags, this patch is a NFC. With this patch, the targets have to just pass FrameSetup/FrameDestroy flag to the storeRegToStackSlot/loadRegFromStackSlot calls from the target derived spillCalleeSavedRegisters and restoreCalleeSavedRegisters to set those flags on callee saved register spill/reload instructions. Also, this patch makes it very easy to set the source line information on callee saved register spill/reload instructions which is needed by the DwarfDebug.cpp implementation to set prologue_end and epilogue_begin markers more accurately. As per DwarfDebug.cpp implementation: prologue_end is the first known non-DBG_VALUE and non-FrameSetup location that marks the beginning of the function body epilogue_begin is the first FrameDestroy location that has been seen in the epilogue basic block With this patch, the targets have to just do the following to set the source line information on callee saved register spill/reload instructions, without hampering the LLVM's efforts to avoid adding source line information on the artificial code generated by the compiler. <Foo>InstrInfo::storeRegToStackSlot() { ... DebugLoc DL = Flags & MachineInstr::FrameSetup ? DebugLoc() : MBB.findDebugLoc(I); ... } <Foo>InstrInfo::loadRegFromStackSlot() { ... DebugLoc DL = Flags & MachineInstr::FrameDestroy ? MBB.findDebugLoc(I) : DebugLoc(); ... } While I understand this patch would break out-of-tree backend builds, I think it is in the right direction. One immediate use case that can benefit from this patch is fixing #120553 becomes simpler.

The DwarfDebug.cpp implementation expects the epilogue instructions to have source location of last non-debug instruction after which the epilogue instructions are inserted. The epilogue_begin is set on location of the first FrameDestroy instruction with source line information that has been seen in the epilogue basic block. In the trunk, the risc-v backend sets the epilogue_begin after the epilogue has actually begun i.e. after callee saved register reloads and the source line information is not set on those reload instructions. This is leading to llvm#120553 where, while debugging, breaking on or single stepping to the epilogue_begin location will make accessing the variables from wrong place as the FP has been restored to the parent frame's FP. To fix that, this patch sets FrameSetup/FrameDestroy flags on the callee saved register spill/reload instructions which is actually correct. Then the RISCVInstrInfo::loadRegFromStackSlot uses FrameDestroy flag to identify a reload of the callee saved register in the epilogue and copies the source line information from insert position instruction to that reload instruction. Requires cce55a1 Fixes llvm#120553

The DwarfDebug.cpp implementation expects the epilogue instructions to have source location of last non-debug instruction after which the epilogue instructions are inserted. The epilogue_begin is set on location of the first FrameDestroy instruction with source line information that has been seen in the epilogue basic block. In the trunk, the risc-v backend sets the epilogue_begin after the epilogue has actually begun i.e. after callee saved register reloads and the source line information is not set on those reload instructions. This is leading to llvm#120553 where, while debugging, breaking on or single stepping to the epilogue_begin location will make accessing the variables from wrong place as the FP has been restored to the parent frame's FP. To fix that, this patch sets FrameSetup/FrameDestroy flags on the callee saved register spill/reload instructions which is actually correct. Then the RISCVInstrInfo::loadRegFromStackSlot uses FrameDestroy flag to identify a reload of the callee saved register in the epilogue and copies the source line information from insert position instruction to that reload instruction. Requires PR llvm#120622 Fixes llvm#120553

…20623) The DwarfDebug.cpp implementation expects the epilogue instructions to have source location of last non-debug instruction after which the epilogue instructions are inserted. The epilogue_begin is set on location of the first FrameDestroy instruction with source line information that has been seen in the epilogue basic block. In the trunk, the risc-v backend sets the epilogue_begin after the epilogue has actually begun i.e. after callee saved register reloads and the source line information is not set on those reload instructions. This is leading to #120553 where, while debugging, breaking on or single stepping to the epilogue_begin location will make accessing the variables from wrong place as the FP has been restored to the parent frame's FP. To fix that, this patch sets FrameSetup/FrameDestroy flags on the callee saved register spill/reload instructions which is actually correct. Then the RISCVInstrInfo::loadRegFromStackSlot uses FrameDestroy flag to identify a reload of the callee saved register in the epilogue and copies the source line information from insert position instruction to that reload instruction. Requires PR #120622 Fixes #120553

RamNalamothu added backend:RISC-V bug Indicates an unexpected problem or unintended behavior debuginfo new issue labels Dec 19, 2024

RamNalamothu self-assigned this Dec 19, 2024

EugeneZelenko removed bug Indicates an unexpected problem or unintended behavior new issue labels Dec 19, 2024

RamNalamothu mentioned this issue Dec 19, 2024

[llvm] Pass MachineInstr flags to storeRegToStackSlot/loadRegFromStackSlot (NFC) #120622

Merged

This was linked to pull requests Dec 19, 2024

[RISC-V] Fix incorrect epilogue_begin #120623

Merged

[llvm] Pass MachineInstr flags to storeRegToStackSlot/loadRegFromStackSlot (NFC) #120622

Merged

RamNalamothu mentioned this issue Dec 20, 2024

[RISC-V] Fix incorrect epilogue_begin #120623

Merged

RamNalamothu closed this as completed in #120622 Jan 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[RISC-V] epilogue_begin is set incorrectly #120553

[RISC-V] epilogue_begin is set incorrectly #120553

RamNalamothu commented Dec 19, 2024

llvmbot commented Dec 19, 2024

Uh oh!

llvmbot commented Dec 19, 2024

Uh oh!

llvmbot commented Dec 19, 2024

Uh oh!

RamNalamothu commented Dec 19, 2024

Uh oh!

wangpc-pp commented Dec 19, 2024 •

edited

Loading

Uh oh!

RamNalamothu commented Dec 19, 2024

Uh oh!

[RISC-V] epilogue_begin is set incorrectly #120553

[RISC-V] epilogue_begin is set incorrectly #120553

Comments

RamNalamothu commented Dec 19, 2024

llvmbot commented Dec 19, 2024

Uh oh!

llvmbot commented Dec 19, 2024

Uh oh!

llvmbot commented Dec 19, 2024

Uh oh!

RamNalamothu commented Dec 19, 2024

Uh oh!

wangpc-pp commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RamNalamothu commented Dec 19, 2024

Uh oh!

wangpc-pp commented Dec 19, 2024 •

edited

Loading