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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

My apologies that I did not try cfi-inserter-basic-block-sections-callee-save-registers.ll
The generated assembly does have .cfi_offset
(I cannot apply the patch with `arc patch` (https://reviews.llvm.org/D79978?id=277216#2138208 )
so I used curl | patch and somehow ignored the locally modified .ll)

However, I do think inline asm with clobbered register list () has some advantage: the .cfi_offset registers
can be precisely controlled.

  call void asm sideeffect "nop", "~{rbp},~{r12},~{r13},~{r14},~{r15}"()

The CHECK lines can thus be strengthened:

  ; CFI_INSTR:      CFI_INSTRUCTION def_cfa_offset 48
  ; CFI_INSTR-NEXT: CFI_INSTRUCTION offset $r12, -48
  ; CFI_INSTR-NEXT: CFI_INSTRUCTION offset $r13, -40
  ; CFI_INSTR-NEXT: CFI_INSTRUCTION offset $r14, -32
  ; CFI_INSTR-NEXT: CFI_INSTRUCTION offset $r15, -24
  ; CFI_INSTR-NEXT: CFI_INSTRUCTION offset $rbp, -16

While the current approach cannot control the used registers.

----

There is another unaddressed comment.

Adding part of your write-up to the description will be helpful
https://reviews.llvm.org/D79978?id=277216#2145979



================
Comment at: llvm/test/CodeGen/X86/cfi-basic-block-sections-1.ll:3
+; RUN: llc -O0 %s --basicblock-sections=all -mtriple=x86_64-unknown-linux-gnu -filetype=asm --frame-pointer=none -o - | FileCheck --check-prefix=SECTIONS_NOFP_CFI %s
+; RUN: llc -O0 %s --basicblock-sections=all -mtriple=x86_64-unknown-linux-gnu -filetype=obj --frame-pointer=all -o - | llvm-dwarfdump --debug-frame  - | FileCheck --check-prefix=DEBUG_FRAME %s
+
----------------
While `--eh-frame` is an alias for `--debug-frame`, I think using `--eh-frame` here is more appropriate. This tests .eh_frame, not .debug_frame.



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

https://reviews.llvm.org/D79978