[AsmPrinterDwarf] Add support for .cfi_restore directive

[thegameg]

Summary

As of today we only use .cfi_offset to specify the offset of a CSR, but we never use .cfi_restore when the CSR is restored.

If we want to perform a more advanced type of shrink-wrapping, we need to use .cfi_restore in order to switch the CFI state between blocks.

This patch only aims at adding support for the directive.

[aprantl]

Do you have a testcase for this?

[thegameg]

• Add MIR support (printing and parsing).
• Add MIR test.
• Update and rename CodeGen test.
• Add MC test.

[probinson]

lib/CodeGen/MIRPrinter.cpp

1214 ↗

(On Diff #109023)

I didn't see a test for this bit here.

test/CodeGen/AArch64/cfi_restore.mir

4 ↗

(On Diff #109023)

Using CHECK-LABEL would seem to be unnecessary.
If it is necessary, using it with such a short name and no punctuation or other context is prone to matching something you didn't expect.

test/CodeGen/MIR/AArch64/cfi.mir

22 ↗

(On Diff #109023)

CHECK-LABEL: name: trivial_fp_func

38 ↗

(On Diff #109023)

CHECK-LABEL: name: trivial_fp_func_restore

test/MC/AArch64/cfi.s

4 ↗

(On Diff #109023)

Remove unnecessary CHECK-LABEL

[thegameg]

Add / remove CHECK-LABELs.

[thegameg]

lib/CodeGen/MIRPrinter.cpp

1214 ↗

(On Diff #109023)

The MIRParser doesn't attach any symbols to the MCCFIInstructions it creates (lib/CodeGen/MIRParser/MIParser.cpp:MIParser::parseCFIOperand, and since we don't emit any .cfi_restore directives yet, I can't add a test for this.

[probinson]

I'm happy with the test changes and the code looks straightforward enough. LGTM.

llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

case MCCFIInstruction::OpSameValue:
     OutStreamer->EmitCFISameValue(Inst.getRegister());
     break;
   case MCCFIInstruction::OpGnuArgsSize:
     OutStreamer->EmitCFIGnuArgsSize(Inst.getOffset());
     break;
   case MCCFIInstruction::OpEscape:
     OutStreamer->EmitCFIEscape(Inst.getValues());
     break;
+  case MCCFIInstruction::OpRestore:
+    OutStreamer->EmitCFIRestore(Inst.getRegister());
+    break;
   }
 }
 
 void AsmPrinter::emitDwarfDIE(const DIE &Die) const {
   // Emit the code (index) for the abbreviation.
   if (isVerbose())
     OutStreamer->AddComment("Abbrev [" + Twine(Die.getAbbrevNumber()) + "] 0x" +
                             Twine::utohexstr(Die.getOffset()) + ":0x" +

llvm/trunk/lib/CodeGen/MIRParser/MILexer.h

enum TokenKind {
     kw_tied_def,
     kw_frame_setup,
     kw_debug_location,
     kw_cfi_same_value,
     kw_cfi_offset,
     kw_cfi_def_cfa_register,
     kw_cfi_def_cfa_offset,
     kw_cfi_def_cfa,
+    kw_cfi_restore,
     kw_blockaddress,
     kw_intrinsic,
     kw_target_index,
     kw_half,
     kw_float,
     kw_double,
     kw_x86_fp80,
     kw_fp128,

llvm/trunk/lib/CodeGen/MIRParser/MILexer.cpp

return StringSwitch<MIToken::TokenKind>(Identifier)
       .Case("tied-def", MIToken::kw_tied_def)
       .Case("frame-setup", MIToken::kw_frame_setup)
       .Case("debug-location", MIToken::kw_debug_location)
       .Case("same_value", MIToken::kw_cfi_same_value)
       .Case("offset", MIToken::kw_cfi_offset)
       .Case("def_cfa_register", MIToken::kw_cfi_def_cfa_register)
       .Case("def_cfa_offset", MIToken::kw_cfi_def_cfa_offset)
       .Case("def_cfa", MIToken::kw_cfi_def_cfa)
+      .Case("restore", MIToken::kw_cfi_restore)
       .Case("blockaddress", MIToken::kw_blockaddress)
       .Case("intrinsic", MIToken::kw_intrinsic)
       .Case("target-index", MIToken::kw_target_index)
       .Case("half", MIToken::kw_half)
       .Case("float", MIToken::kw_float)
       .Case("double", MIToken::kw_double)
       .Case("x86_fp80", MIToken::kw_x86_fp80)
       .Case("fp128", MIToken::kw_fp128)

llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp

bool MIParser::parseCFIOperand(MachineOperand &Dest) {
   case MIToken::kw_cfi_def_cfa:
     if (parseCFIRegister(Reg) || expectAndConsume(MIToken::comma) ||
         parseCFIOffset(Offset))
       return true;
     // NB: MCCFIInstruction::createDefCfa negates the offset.
     CFIIndex =
         MF.addFrameInst(MCCFIInstruction::createDefCfa(nullptr, Reg, -Offset));
     break;
+  case MIToken::kw_cfi_restore:
+    if (parseCFIRegister(Reg))
+      return true;
+    CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestore(nullptr, Reg));
+    break;
+
   default:
     // TODO: Parse the other CFI operands.
     llvm_unreachable("The current token should be a cfi operand");
   }
   Dest = MachineOperand::CreateCFIIndex(CFIIndex);
   return false;
 }
 
bool MIParser::parseMachineOperand(MachineOperand &Dest,
   case MIToken::md_diexpr:
   case MIToken::exclaim:
     return parseMetadataOperand(Dest);
   case MIToken::kw_cfi_same_value:
   case MIToken::kw_cfi_offset:
   case MIToken::kw_cfi_def_cfa_register:
   case MIToken::kw_cfi_def_cfa_offset:
   case MIToken::kw_cfi_def_cfa:
+  case MIToken::kw_cfi_restore:
     return parseCFIOperand(Dest);
   case MIToken::kw_blockaddress:
     return parseBlockAddressOperand(Dest);
   case MIToken::kw_intrinsic:
     return parseIntrinsicOperand(Dest);
   case MIToken::kw_target_index:
     return parseTargetIndexOperand(Dest);
   case MIToken::kw_liveout:

llvm/trunk/lib/CodeGen/MIRPrinter.cpp

case MCCFIInstruction::OpDefCfaOffset:
     break;
   case MCCFIInstruction::OpDefCfa:
     OS << "def_cfa ";
     if (CFI.getLabel())
       OS << "<mcsymbol> ";
     printCFIRegister(CFI.getRegister(), OS, TRI);
     OS << ", " << CFI.getOffset();
     break;
+  case MCCFIInstruction::OpRestore:
+    OS << "restore ";
+    if (CFI.getLabel())
+      OS << "<mcsymbol> ";
+    printCFIRegister(CFI.getRegister(), OS, TRI);
+    break;
   default:
     // TODO: Print the other CFI Operations.
     OS << "<unserializable cfi operation>";
     break;
   }
 }
 
 void llvm::printMIR(raw_ostream &OS, const Module &M) {
   yaml::Output Out(OS);
   Out << const_cast<Module &>(M);
 }
 
 void llvm::printMIR(raw_ostream &OS, const MachineFunction &MF) {
   MIRPrinter Printer(OS);
   Printer.print(MF);
 }

llvm/trunk/lib/MC/MCAsmStreamer.cpp

public:
   void EmitCFIDefCfa(int64_t Register, int64_t Offset) override;
   void EmitCFIDefCfaOffset(int64_t Offset) override;
   void EmitCFIDefCfaRegister(int64_t Register) override;
   void EmitCFIOffset(int64_t Register, int64_t Offset) override;
   void EmitCFIPersonality(const MCSymbol *Sym, unsigned Encoding) override;
   void EmitCFILsda(const MCSymbol *Sym, unsigned Encoding) override;
   void EmitCFIRememberState() override;
   void EmitCFIRestoreState() override;
+  void EmitCFIRestore(int64_t Register) override;
   void EmitCFISameValue(int64_t Register) override;
   void EmitCFIRelOffset(int64_t Register, int64_t Offset) override;
   void EmitCFIAdjustCfaOffset(int64_t Adjustment) override;
   void EmitCFIEscape(StringRef Values) override;
   void EmitCFIGnuArgsSize(int64_t Size) override;
   void EmitCFISignalFrame() override;
   void EmitCFIUndefined(int64_t Register) override;
   void EmitCFIRegister(int64_t Register1, int64_t Register2) override;

 }
 
 void MCAsmStreamer::EmitCFIRestoreState() {
   MCStreamer::EmitCFIRestoreState();
   OS << "\t.cfi_restore_state";
   EmitEOL();
 }
 
+void MCAsmStreamer::EmitCFIRestore(int64_t Register) {
+  MCStreamer::EmitCFIRestore(Register);
+  OS << "\t.cfi_restore ";
+  EmitRegisterName(Register);
+  EmitEOL();
+}
+
 void MCAsmStreamer::EmitCFISameValue(int64_t Register) {
   MCStreamer::EmitCFISameValue(Register);
   OS << "\t.cfi_same_value ";
   EmitRegisterName(Register);
   EmitEOL();
 }
 
 void MCAsmStreamer::EmitCFIRelOffset(int64_t Register, int64_t Offset) {

llvm/trunk/test/CodeGen/AArch64/cfi_restore.mir

+# RUN: llc -mtriple=aarch64-- -start-after prologepilog -o - %s | FileCheck %s
+---
+name:            fun
+# CHECK: .cfi_startproc
+tracksRegLiveness: true
+frameInfo:
+  stackSize:       16
+  maxAlignment:    8
+  hasCalls:        true
+stack:
+  - { id: 0, type: spill-slot, offset: -8, size: 8, alignment: 8, stack-id: 0,
+      callee-saved-register: '%lr' }
+  - { id: 1, type: spill-slot, offset: -16, size: 8, alignment: 8, stack-id: 0,
+      callee-saved-register: '%fp' }
+body:             |
+  bb.0:
+    liveins: %fp, %lr
+
+    %sp = frame-setup SUBXri %sp, 16, 0
+    frame-setup STRXui killed %fp, %sp, 0 :: (store 8 into %stack.1)
+    frame-setup CFI_INSTRUCTION offset %w29, -16
+    ; CHECK: .cfi_offset w29, -16
+    frame-setup STRXui killed %lr, %sp, 1 :: (store 8 into %stack.0)
+    frame-setup CFI_INSTRUCTION offset %w30, -8
+    ; CHECK: .cfi_offset w30, -8
+    %fp = frame-setup ADDXri %sp, 0, 0
+    frame-setup CFI_INSTRUCTION def_cfa %w29, 16
+    %lr = LDRXui %sp, 1 :: (load 8 from %stack.0)
+    CFI_INSTRUCTION restore %w30
+    ; CHECK: .cfi_restore w30
+    %fp = LDRXui %sp, 0 :: (load 8 from %stack.1)
+    CFI_INSTRUCTION restore %w29
+    ; CHECK: .cfi_restore w29
+    %sp = ADDXri %sp, 16, 0
+    RET_ReallyLR
+    ; CHECK: .cfi_endproc
+...

llvm/trunk/test/CodeGen/MIR/AArch64/cfi-def-cfa.mir

-# RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass none -o - %s | FileCheck %s
-# This test ensures that the MIR parser parses the def_cfa operands
-# correctly.
-
---- |
-
-  declare void @foo()
-
-  define void @trivial_fp_func() {
-  entry:
-    call void @foo()
-    ret void
-  }
-
-...
----
-name:            trivial_fp_func
-body: |
-  bb.0.entry:
-    liveins: %lr, %fp, %lr, %fp
-
-    %sp = frame-setup STPXpre killed %fp, killed %lr, %sp, -2
-    %fp = frame-setup ADDXri %sp, 0, 0
-    ; CHECK: CFI_INSTRUCTION def_cfa %w29, 16
-    frame-setup CFI_INSTRUCTION def_cfa %w29, 16
-    frame-setup CFI_INSTRUCTION offset %w30, -8
-    frame-setup CFI_INSTRUCTION offset %w29, -16
-    BL @foo, csr_aarch64_aapcs, implicit-def dead %lr, implicit %sp, implicit-def %sp
-    %sp, %fp, %lr = LDPXpost %sp, 2
-    RET_ReallyLR
-...

llvm/trunk/test/CodeGen/MIR/AArch64/cfi.mir

+# RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass none -o - %s | FileCheck %s
+# This test ensures that the MIR parser parses the cfi directives correctly.
+
+--- |
+
+  declare void @foo()
+
+  define void @trivial_fp_func() {
+  entry:
+    call void @foo()
+    ret void
+  }
+
+  define void @trivial_fp_func_restore() {
+  entry:
+    call void @foo()
+    ret void
+  }
+
+...
+---
+name:            trivial_fp_func
+# CHECK-LABEL: name: trivial_fp_func
+body: |
+  bb.0.entry:
+    liveins: %lr, %fp, %lr, %fp
+
+    %sp = frame-setup STPXpre killed %fp, killed %lr, %sp, -2
+    %fp = frame-setup ADDXri %sp, 0, 0
+    ; CHECK: CFI_INSTRUCTION def_cfa %w29, 16
+    frame-setup CFI_INSTRUCTION def_cfa %w29, 16
+    frame-setup CFI_INSTRUCTION offset %w30, -8
+    frame-setup CFI_INSTRUCTION offset %w29, -16
+    BL @foo, csr_aarch64_aapcs, implicit-def dead %lr, implicit %sp, implicit-def %sp
+    %sp, %fp, %lr = LDPXpost %sp, 2
+    RET_ReallyLR
+...
+---
+name:            trivial_fp_func_restore
+# CHECK-LABEL: name: trivial_fp_func_restore
+body: |
+  bb.0.entry:
+    liveins: %lr, %fp
+
+    %sp = frame-setup STPXpre killed %fp, killed %lr, %sp, -2
+    %fp = frame-setup ADDXri %sp, 0, 0
+    frame-setup CFI_INSTRUCTION def_cfa %w29, 16
+    frame-setup CFI_INSTRUCTION offset %w30, -8
+    ; CHECK: CFI_INSTRUCTION offset %w30, -8
+    frame-setup CFI_INSTRUCTION offset %w29, -16
+    ; CHECK: CFI_INSTRUCTION offset %w29, -16
+    BL @foo, csr_aarch64_aapcs, implicit-def dead %lr, implicit %sp, implicit-def %sp
+    %sp, %fp, %lr = LDPXpost %sp, 2
+    CFI_INSTRUCTION restore %w30
+    ; CHECK: CFI_INSTRUCTION restore %w30
+    CFI_INSTRUCTION restore %w29
+    ; CHECK: CFI_INSTRUCTION restore %w29
+    RET_ReallyLR
+...

llvm/trunk/test/MC/AArch64/cfi.s

+// RUN: llvm-mc -triple aarch64-- -o - %s | FileCheck %s
+
+fun:
+  .cfi_startproc
+// CHECK: .cfi_startproc
+  stp  x29, x30, [sp, #-16]!
+.Lcfi0:
+  .cfi_offset w29, -16
+// CHECK: .cfi_offset w29, -16
+.Lcfi1:
+  .cfi_offset w30, -8
+// CHECK: .cfi_offset w30, -8
+  mov   x29, sp
+.Lcfi2:
+  .cfi_def_cfa w29, 16
+// CHECK: .cfi_def_cfa w29, 16
+.Lcfi3:
+  .cfi_restore w30
+// CHECK: .cfi_restore w30
+  ldr  x29, [sp], #16
+.Lcfi4:
+  .cfi_restore w29
+// CHECK: .cfi_restore w29
+  ret
+  .cfi_endproc
+// CHECK: .cfi_endproc