[PATCH] D78778: [AsmPrinter] Fix emitting CFI for debug when exceptions are not supported

[Venkata Ramanaiah Nalamothu via Phabricator via llvm-commits]

RamNalamothu added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:378
 
+  /// Currently emitting CFI unwind information is entangled with supporting
+  /// the exceptions. When ExceptionsType == ExceptionHandling::None, this
----------------
MaskRay wrote:
> "Currently emitting CFI unwind information is entangled with supporting ..." This is a remark. It has minor state compared with the main comment. It can be added at the end or omitted.
Doesn't my original sequence of statements clearly state why and for what `usesCFIForDebug()` is needed given the current state of things on if/when a .eh_frame or .debug_frame is emitted?


================
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:379
+  /// Currently emitting CFI unwind information is entangled with supporting
+  /// the exceptions. When ExceptionsType == ExceptionHandling::None, this
+  /// returns true if DWARF CFI unwind information should be emitted either
----------------
MaskRay wrote:
> `ExceptionsType` is not mentioned.
> `ExceptionsType` is not mentioned.

Sorry, I didn't understand that.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:351
   switch (MAI->getExceptionHandlingType()) {
+  // We may want to emit CFI for debug or when `ForceDwarfFrameSection`.
+  case ExceptionHandling::None:
----------------
MaskRay wrote:
> This comment is not clear. Does it only apply to `None`?
Yes, will change it to

```
case ExceptionHandling::None:
    // We may want to emit CFI for debug or when `ForceDwarfFrameSection`.
    LLVM_FALLTHROUGH;
case ExceptionHandling::SjLj:
```


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1067
+bool AsmPrinter::usesCFIForDebug() const {
+  return MAI->getExceptionHandlingType() == ExceptionHandling::None &&
+         ((MMI->hasDebugInfo() && ModuleCFISection == CFISection::Debug) ||
----------------
MaskRay wrote:
> Does AMDGPU use `MAI->getExceptionHandlingType() == ExceptionHandling::None` for .debug_frame?
> 
> Why can't it use `DwarfCFI`?
> Does AMDGPU use `MAI->getExceptionHandlingType() == ExceptionHandling::None` for .debug_frame?

Yes and AMDGPU can't afford to have an `.eh_frame`, which is an `ALLOC` section, for debug purpose.

> Why can't it use `DwarfCFI`?

As mentioned earlier, AMDGPU don't support exceptions.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78778