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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Last few nits



================
Comment at: llvm/test/CodeGen/X86/cfi-basic-block-sections-1.ll:1
+; RUN: llc -O0 %s --basicblock-sections=all -mtriple=x86_64-unknown-linux-gnu -filetype=asm --frame-pointer=all -o - | FileCheck --check-prefix=SECTIONS_CFI %s
+; 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
----------------
x86_64-unknown-linux-gnu -> x86_64

This tests generic ELF behavior, not specific to linux-gnu.


================
Comment at: llvm/test/CodeGen/X86/cfi-basic-block-sections-1.ll:49
+
+; There must be 1 CIE and 3 FDEs
+
----------------
Comments tend to be full sentences (with period).

You may consider `;; ` for tests. IIRC update_llc_test_checks.py can sometimes drop comments. We can teach update_llc_test_checks.py to retain comments with `;; `


================
Comment at: llvm/test/CodeGen/X86/cfi-inserter-basic-block-sections-callee-save-registers.ll:7
+; SECTIONS_CFI:      .cfi_offset %rbp, -16
+; SECTIONS_CFI:      .cfi_offset %r{{..}}, -56
+; SECTIONS_CFI-NEXT: .cfi_offset %r{{..}}, -48
----------------
You can capture the register name with `[[RA:%r.+]]`

and use `[[RA]]` below to reference the captured name.


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

https://reviews.llvm.org/D79978