This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Begin emitting CFI for AMDGCN
AbandonedPublic

Authored by scott.linder on Mar 26 2020, 12:19 PM.

Details

Summary

Set SupportsDebugUnwindInformation in AMDGPUMCAsmInfo.

Diff Detail

Event Timeline

scott.linder created this revision.Mar 26 2020, 12:19 PM
arsenm added inline comments.Mar 26 2020, 1:26 PM
llvm/test/CodeGen/AMDGPU/branch-relaxation.ll
257 ↗(On Diff #252939)

Why does it matter in any of these cases?

llvm/test/DebugInfo/AMDGPU/cfi.ll
23

This is the deprecated attribute. This should use frame-pointer=

llvm/test/MC/ELF/AMDGPU/lit.local.cfg
3

Copy paste X86

scott.linder marked 3 inline comments as done.Mar 26 2020, 2:12 PM
scott.linder added inline comments.
llvm/test/CodeGen/AMDGPU/branch-relaxation.ll
257 ↗(On Diff #252939)

When we say we are ExceptionHandling::DwarfCFI we are saying that we want an .eh_frame for any functions which are not nounwind (i.e. anything that can throw) and that we want a .debug_frame section for anything else that has debug info enabled. We never need the .eh_frame case, and these tests are agnostic to it anyway, so it seemed nicer to just update all tests to be nounwind than to add CFI noise to them all.

There is currently no way for a target to say "I only ever want CFI for debugging purposes, because I don't support exceptions anyway". I didn't go about adding one because we always mark functions nounwind anyway, and if we ever stopped it would likely be because we intend to support EH and would want the .eh_frame to start being generated.

llvm/test/DebugInfo/AMDGPU/cfi.ll
23

This test could use a bit of cleanup, I don't think it depends on this attribute anyway. I'll trim it down to just what is actually relevant.

llvm/test/MC/ELF/AMDGPU/lit.local.cfg
3

I think this X86 is actually correct here, the root llvm/test/MC/ELF dir is already predicated (there is no X86 subdir).

arsenm accepted this revision.Mar 26 2020, 2:20 PM
This revision is now accepted and ready to land.Mar 26 2020, 2:20 PM

Remove unused attribute

arsenm accepted this revision.Mar 26 2020, 2:45 PM

Rather than lie about supporting exceptions to get unwind information, add an
option to support debug-only unwind information.

Add tests to ensure we are generating CU and CIE augmentation strings.

scott.linder edited the summary of this revision. (Show Details)

Rebase and update summary

MaskRay added inline comments.May 12 2020, 1:44 PM
llvm/test/DebugInfo/AMDGPU/cfi.ll
14

Maybe CHECK-COUNT-6?

llvm/test/MC/ELF/AMDGPU/lit.local.cfg
4

not in

MaskRay added inline comments.May 12 2020, 1:46 PM
llvm/test/MC/ELF/AMDGPU/cfi.s
21

Consider llvm-readelf if llvm-readobj output is too long.

lld/test/ELF has some examples, e.g.

# RO:      Name         Type     Address          Off    Size   ES Flg Lk Inf Al
# RO-NEXT:              NULL     0000000000000000 000000 000000 00      0   0  0
# RO-NEXT: .data        PROGBITS 0000000000000000 {{.*}} 00000c 00  WA  0   0  8
``
MaskRay added inline comments.May 12 2020, 1:47 PM
llvm/test/MC/ELF/AMDGPU/cfi.s
2

Most assembly tests can use # (shorter than //)

scott.linder marked 4 inline comments as done.
  • Use -COUNT-N form
  • Use '#' comment character
  • Use llvm-readelf for more compact tests
  • Refactor lit config test
llvm/test/DebugInfo/AMDGPU/cfi.ll
14

Thank you, done!

Rebase and update tests

Rebase onto LLVM master

greened accepted this revision.Sep 30 2020, 1:31 PM
greened added inline comments.
llvm/test/MC/ELF/AMDGPU/cfi.s
14

Are these intended to be fixed before merge or is the test just nothing something should be fixed? If the latter, perhaps we need an XFAIL test for it? But maybe it's not worth it.

RamNalamothu added inline comments.Oct 20 2020, 1:51 AM
llvm/test/MC/ELF/AMDGPU/cfi.s
14

Currently the AsmParser is not skipping the '\n' i.e. EndOfStatement at the end while parsing the asm directives and hence the extra new line is being generated for that '\n'.

And the above behavior is not a result of any of the patch in this patch series.
May be the FIXME here can be changed to a note indicating that the blank line is required and/or expected.

Rebase onto LLVM Master