[PATCH] D79978: Call Frame Information (CFI) Handling for Basic Block Sections

[Sriraman Tallam via Phabricator via llvm-commits]

tmsriram added a comment.

In D79978#2146011 <https://reviews.llvm.org/D79978#2146011>, @MaskRay wrote:

> In D79978#2146009 <https://reviews.llvm.org/D79978#2146009>, @amharc wrote:
>
> > In D79978#2145979 <https://reviews.llvm.org/D79978#2145979>, @MaskRay wrote:
> >
> > > I cannot find a test with `.cfa_offset` or describing a non-rbp register, so the implementation is under-tested. How about leveraging inline asm to clobber callee-saved registers?
> >
> >
> > I'm not exactly sure which CFI instruction this comment refers to (there is no `.cfa_offset` directive, see e.g. https://sourceware.org/binutils/docs/as/CFI-directives.html).
> >
> > If to `.cfi_def_cfa_offset` - the question was answered before: we emit a full CFA definition (`.cfi_def_cfa` - setting both the offset and the register) which is equivalent to a `.cfi_def_cfa_offset` (only setting the offset) plus a `.cfi_def_cfa_register` (only setting the register). Moreover, for code following the SysV ABI the register used in the `.cfi_def_cfa_register`/`.cfi_def_cfa` instructions is conventionally always set to either `rbp` (if the frame pointer is kept explicitly) or in `rsp` (when it's omitted). Both cases are covered by the existing tests. We are not aware of any easy way to force a different register to be the frame pointer.
> >
> > If the review comment above refers to `.cfi_offset` instead, please note that the `cfi-basic-block-sections-1.ll` test checks that `.cfi_offset` is emitted properly in the generated assembly and `cfi-inserter-basic-block-sections-callee-save-registers.ll` checks that the CFI Inserter Pass actually inserts the corresponding `CFI_INSTRUCTION offset` instructions for clobbered callee-saved registers. I'm not exactly sure what would be gained by using inline assembly instead.
>
>
> I ran llc for both tests. Neither has a section with CFI describing a non-rbp register at the top, thus I say I am not sure the implementation is properly tested.


What is the problem here, I dont follow.  Is this with or without frame pointer?  If you ran with O2 <https://reviews.llvm.org/owners/package/2/> frame pointer is omitted so you must explicitly say --frame-pointer=all.


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

https://reviews.llvm.org/D79978