[PATCH] D78778: [AMDGPU] Add SupportsDebugUnwindInformation to MCAsmInfo

[Venkata Ramanaiah Nalamothu via Phabricator via llvm-commits]

RamNalamothu added a comment.

In D78778#2444445 <https://reviews.llvm.org/D78778#2444445>, @dblaikie wrote:

> (I'm probably not the best person to review this - just chimed in on the patch structure/testing issues, but here are some naive questions)
>
> In D78778#2301155 <https://reviews.llvm.org/D78778#2301155>, @RamNalamothu wrote:
>
>> In D78778#2300827 <https://reviews.llvm.org/D78778#2300827>, @probinson wrote:
>>
>>> I haven't touched the streamer side of AsmPrinter in a long time, but...
>>> On PS4 we default to `-fno-exceptions` and while the section ends up named `.eh_frame` instead of `.debug_frame` the content is fine; we've had no complaints from our debugger folks.  Aside from the introduction of the .cfi directives (which you do want), I'm not aware that there's any codegen consequence to this combination.
>>> So, I'm not clear why a special streamer is really necessary.
>>
>> A special streamer is a straight forward and minimal changes approach we could arrive at as a means to solve the following concerns observed with the current implementation.
>>
>> 1. Today, with //`-fno-exceptions`//, the command line option //`--force-dwarf-frame-section`// of llc doesn't do what it should as per D67216 <https://reviews.llvm.org/D67216> i.e. a  //`.debug_frame`// is not generated
>
> Does this change address (1)? Should there be a test for `--force-dwarf-frame-section` to demonstrate this fix? (what cases did D67216 <https://reviews.llvm.org/D67216> address? that are distinct from this case that it misses)

The fact that the unwind information could be generated for debug or exception handling use case is correctly modeled in //`MachineFunction::needsFrameMoves()`// and //`AsmPrinter::needsCFIMoves()`//, and forcing to generate //`.debug_frame`// is honored as expected here, but the issue is arising as the unwind information generation is implemented in //`class DwarfCFIException`// through the //`class EHStreamer`// base class and this streamer is added to AsmPrinter debug handlers <https://reviews.llvm.org/source/llvm-github/browse/main/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp$359> only when exceptions are enabled and all the scenarios in //`MachineFunction::needsFrameMoves()`/////`AsmPrinter::needsCFIMoves()`// are not considered here to decide when/what unwind information tables is to be generated. This patch considers all those scenarios while generating unwind tables (please have a look at //llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:339// of this patch).

Yeah, a test case could be added to demonstrate that this patch fix the AsmPrinter behavior to get //`.debug_frame`// generated for //`--force-dwarf-frame-section`// even when //`-fno-exceptions`// is specified.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78778/new/

https://reviews.llvm.org/D78778