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

[Sriraman Tallam via Phabricator via llvm-commits]

tmsriram added a comment.

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

> In D79978#2141768 <https://reviews.llvm.org/D79978#2141768>, @wmi wrote:
>
> > In D79978#2140849 <https://reviews.llvm.org/D79978#2140849>, @MaskRay wrote:
> >
> > > I haven't looked into the details, but the test suggests that the patch is wrong:
> > >
> > >   # basic-block-sections-cfi-1.ll
> > >           .section        .text,"ax", at progbits,unique,2
> > >   _Z2f3b.2:                               # %if.end
> > >           .cfi_startproc
> > >           .cfi_def_cfa %rbp, 16    # this should be inserted after addq $16, %rsp
> > >           .cfi_offset %rbp, -16    # this should be after .cfi_def_cfa %rbp, 16
> > >           addq    $16, %rsp
> > >           popq    %rbp
> > >           .cfi_def_cfa %rsp, 8
> > >           retq
> > >
> >
> >
> > I think the position where the cfi directives are currently inserted is correct. Those directives at the beginning of BB are not to maintain call frame information for instructions inside of BB like "addq    $16, %rsp" and "popq %rbp", but to setup the call frame information correctly at the beginning of BB because the BB could be moved around.
>
>
> Ack. Then what instructions should be placed at the top of these basic blocks? Should `.cfi_def_cfa_register %rbp` be placed as well? If you move these basic blocks around, `.cfi_def_cfa_register %rbp` is currently not tracked.
>
> > For basic-block-sections-cfiinstr_1.ll, have you considered places like CodeGen/X86/cfi-inserter-*? You may even create a subdirectory there.
>
> This question still stands. I haven't seen other CFI tests in`DebugInfo/X86/`


Sure, I will move it, np.


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

https://reviews.llvm.org/D79978