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

[Wei Mi via Phabricator via llvm-commits]

wmi 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.


That is because .cfi_def_cfa %rbp, 16 is identical to the following:
.cfi_def_cfa_register %rbp
.cfi_def_cfa_offset 16


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

https://reviews.llvm.org/D79978