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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D78778#2449379 <https://reviews.llvm.org/D78778#2449379>, @scott.linder wrote:

> In D78778#2449342 <https://reviews.llvm.org/D78778#2449342>, @MaskRay wrote:
>
>> I think the thing we cannot get past is: `ExceptionHandling::DwarfCFI` is called exception handling and thus we expect it to be only useful with C++ (or other language) exceptions. However, the practice is that we overload it for non-exception .eh_frame/.ARM_extab/.ARM_exidx generation as well. So, adding a behavior to `ExceptionHandling::None` and introducing `SupportsDebugUnwindInformation` just seem to bring more complexity to me.
>>
>> If `ExceptionHandling` were called something else, this might be less misleading.
>
> Right, I am open to suggestions, but it also just mirrors the implementation in that the code for doing the debug-only, no-exceptions unwind information is still contained in `EHStreamer`/`DwarfCFIException`. Those could also be renamed, but then most of the code in them is irrelevant to/complicates the debug case.
>
> That's how I ended up at "make a new streamer that is honest about being for debug-only DWARF CFI", and it happened to be nearly empty, so I didn't even have it share code with the exception variants. Maybe instead of an ad-hoc option, though, it could be another `enum DebugUnwindTables { None; DwarfCFI; }`? And rename the `EHStreamer` hierarchy to refer to both exceptions and unwind, and effectively have the same code as in the current patch but with more descriptive names?

Yes, some renaming can help. I believe MCAsmInfo has sufficient states to encode all the needs.

> Maybe instead of an ad-hoc option, though, it could be another enum DebugUnwindTables { None; DwarfCFI; }? And rename the EHStreamer hierarchy to refer to both exceptions and unwind, and effectively have the same code as in the current patch but with more descriptive names?

Having both UnwindTable and ExceptionHandling enums sounds good. `isCFIMoveForDebugging` (the name confused me quite a bit) probably should be folded into the new representations.

> Maybe those aren't present anymore; is anyone aware if that option should affect CodeGen?

I have scanned the call sites of `getExceptionHandlingType`. DwarfEHPrepapre.cpp can replace `resume` with `_Unwind_Resume`. However, if you don't use C++ exceptions, the pass should be a no-op.
There should be no target-independent pass which can affect codegen (wasm and arm are complex and I cannot ensure they don't affect codegen but those are their target decisions)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78778