[PATCH] D78778: [AsmPrinter] Fix emitting CFI for debug when exceptions are not supported
Venkata Ramanaiah Nalamothu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 27 12:29:57 PDT 2021
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
More information about the llvm-commits
mailing list