[MC][MachO]Do not emit DWARF for no-personality case

[oontvoo]

Summary

Detail: Follow up to D144999, where we emitted DWARF for non-canonical personality.

[jyknight]

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

572

Move the change into the isDarwinCanonicalPersonality function?

[oontvoo]

make checks more strict.

[oontvoo]

make checks stricter

[oontvoo]

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

572

well, both of these two predicates are *already* in the function. (it is supposed to tell you if a given symbol is a canonical-darwin personality.)
BUT if it returns false, then it could be any of the following reason:

• (1) the symbol is NULL
• (2) the symbol is NOT MachO
• (3) the symbol is NOT canonical personality symbol.

We only want to return DWARF-MODE here for reason number (3)

If we want to reuse a util function, then we ought to name it isNonNullDarwinNonCanonicalPersonality...
That's a bit hard to read.

[jyknight]

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

572

I'd say the util function should return true for NULL, because NULL is a canonical personality (it's always given index 0, right?)

How do you even get a !MachO symbol in here -- all of this is Mach-O-specific, right? Perhaps can just remove that clause?

[oontvoo]

addressed review comments

[oontvoo]

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

572

Good point.
I've updated the function and replace the isMachO check with just an assert instead. (who knows?!)

[oontvoo]

typo

[tschuett]

llvm/lib/MC/MCAsmBackend.cpp

125 ↗

(On Diff #530035)

Could you please turn the assert into an if-statement?

[jyknight]

llvm/lib/MC/MCAsmBackend.cpp

125 ↗

(On Diff #530035)

Can you please clarify the purpose of your request? Refer to previous discussion thread here as to why this was changed, and if you disagree, please explain why.

[tschuett]

llvm/lib/MC/MCAsmBackend.cpp

125 ↗

(On Diff #530035)

In release mode, the assert cannot fire. It is useless. It only helps you during development.

[oontvoo]

updated patch

llvm/lib/MC/MCAsmBackend.cpp

125 ↗

(On Diff #530035)

This assert is mostly a smoke-check for (most likely) impossible scenario - supposed to help in development only. So I'd have thought an assert is appropriate. Besides, there are plenty of other places both in this file and this package that use 'assert'. I'm not sure how those are different from this instance.

But anyways, changed to llvm_unreachable.

[oontvoo]

rebase

llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

public:
   }
 
   /// Generate the compact unwind encoding from the CFI directives.
   uint32_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
                                          const MCContext *Ctxt) const override {
     ArrayRef<MCCFIInstruction> Instrs = FI->Instructions;
     if (Instrs.empty())
       return CU::UNWIND_ARM64_MODE_FRAMELESS;
-    if (!isDarwinCanonicalPersonality(FI->Personality) &&
+    if (FI->Personality && !isDarwinCanonicalPersonality(FI->Personality) &&
         !Ctxt->emitCompactUnwindNonCanonical())
       return CU::UNWIND_ARM64_MODE_DWARF;
 
     bool HasFP = false;
     unsigned StackSize = 0;
 
     uint32_t CompactUnwindEncoding = 0;
     int CurOffset = 0;

llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

uint32_t ARMAsmBackendDarwin::generateCompactUnwindEncoding(
   DEBUG_WITH_TYPE("compact-unwind", llvm::dbgs() << "generateCU()\n");
   // Only armv7k uses CFI based unwinding.
   if (Subtype != MachO::CPU_SUBTYPE_ARM_V7K)
     return 0;
   // No .cfi directives means no frame.
   ArrayRef<MCCFIInstruction> Instrs = FI->Instructions;
   if (Instrs.empty())
     return 0;
-  if (!isDarwinCanonicalPersonality(FI->Personality) &&
+  if (FI->Personality && !isDarwinCanonicalPersonality(FI->Personality) &&
       !Ctxt->emitCompactUnwindNonCanonical())
     return CU::UNWIND_ARM_MODE_DWARF;
 
   // Start off assuming CFA is at SP+0.
   unsigned CFARegister = ARM::SP;
   int CFARegisterOffset = 0;
   // Mark savable registers as initially unsaved
   DenseMap<unsigned, int> RegOffsets;

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

public:
   }
 
   /// Implementation of algorithm to generate the compact unwind encoding
   /// for the CFI instructions.
   uint32_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
                                          const MCContext *Ctxt) const override {
     ArrayRef<MCCFIInstruction> Instrs = FI->Instructions;
     if (Instrs.empty()) return 0;
-    if (!isDarwinCanonicalPersonality(FI->Personality) &&
+    if (FI->Personality && !isDarwinCanonicalPersonality(FI->Personality) &&
         !Ctxt->emitCompactUnwindNonCanonical())
       return CU::UNWIND_MODE_DWARF;
 
     // Reset the saved registers.
     unsigned SavedRegIdx = 0;
     memset(SavedRegs, 0, sizeof(SavedRegs));
 
     bool HasFP = false;

llvm/test/MC/MachO/AArch64/darwin-ARM64-no-personality.s

+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos11.0 %s | llvm-objdump --unwind-info - | FileCheck %s
+
+# Check that we emit the right encoding for the no-personality case.
+
+# CHECK: Contents of __compact_unwind section:
+# CHECK:  Entry at offset {{.+}}
+# CHECK:    start:                {{.+}} ltmp0
+# CHECK:    length:               {{.+}}
+# CHECK:    compact encoding:     0x02001000
+# CHECK:  Entry at offset {{.+}}
+# CHECK:    start:                {{.+}} __Z3foov
+# CHECK:    length:               {{.+}}
+# CHECK:    compact encoding:     0x04000000
+# CHECK:  Entry at offset {{.+}}
+# CHECK:    start:                {{.+}} _main
+# CHECK:    length:               {{.+}}
+# CHECK:    compact encoding:     0x04000000
+
+
+        .section  __TEXT,__text,regular,pure_instructions
+  .build_version macos, 11, 0
+  .globl  __Z3barPi                     
+  .p2align  2
+__Z3barPi:         
+  .cfi_startproc
+  sub sp, sp, #16
+  .cfi_def_cfa_offset 16
+  str x0, [sp, #8]
+  add sp, sp, #16
+  ret
+  .cfi_endproc
+
+  .globl  __Z3foov           
+  .p2align  2
+__Z3foov:                             
+  .cfi_startproc
+
+  sub sp, sp, #32
+  .cfi_def_cfa_offset 32
+  stp  x29, x30, [sp, #16]   
+  add x29, sp, #16
+  .cfi_def_cfa w29, 16
+  .cfi_offset w30, -8
+  .cfi_offset w29, -16
+  sub x0, x29, #4
+  bl  __Z3barPi
+  ldp  x29, x30, [sp, #16]         
+  add sp, sp, #32
+  ret
+  .cfi_endproc
+                                       
+  .globl  _main                    
+  .p2align  2
+_main:                               
+  .cfi_startproc
+  sub sp, sp, #32
+  .cfi_def_cfa_offset 32
+  stp  x29, x30, [sp, #16]          
+  add x29, sp, #16
+  .cfi_def_cfa w29, 16
+  .cfi_offset w30, -8
+  .cfi_offset w29, -16
+  mov w8, #0                         
+  str w8, [sp, #8]                  
+  stur  wzr, [x29, #-4]
+  bl  __Z3foov
+  ldr w0, [sp, #8]                  
+  ldp  x29, x30, [sp, #16]           
+  add sp, sp, #32
+  ret
+  .cfi_endproc
+
+.subsections_via_symbols

llvm/test/MC/MachO/darwin-x86_64-no-personality.s

+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin10.0 %s | llvm-objdump --unwind-info - | FileCheck %s
+
+# Check that we emit the right encoding for the no-personality case.
+
+# CHECK: Contents of __compact_unwind section:
+# CHECK:   Entry at offset {{.+}}
+# CHECK:     start:                {{.+}} __Z3barPi
+# CHECK:     length:               {{.+}}
+# CHECK:     compact encoding:     0x01000000
+# CHECK:   Entry at offset {{.+}}
+# CHECK:     start:                {{.+}} __Z3foov
+# CHECK:     length:               {{.+}}
+# CHECK:     compact encoding:     0x01000000
+# CHECK:   Entry at offset {{.+}}
+# CHECK:     start:                {{.+}} _main
+# CHECK:     length:               0x1c
+
+        .section  __TEXT,__text,regular,pure_instructions
+  .globl  __Z3barPi        
+  .p2align  4, 0x90
+__Z3barPi:   
+  .cfi_startproc
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+  movq  %rdi, -8(%rbp)
+  popq  %rbp
+  retq
+  .cfi_endproc
+
+  .globl  __Z3foov        
+  .p2align  4, 0x90
+__Z3foov:                        
+  .cfi_startproc
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+  subq  $16, %rsp
+  leaq  -4(%rbp), %rdi
+  callq __Z3barPi
+  addq  $16, %rsp
+  popq  %rbp
+  retq
+  .cfi_endproc
+
+  .globl  _main          
+  .p2align  4, 0x90
+_main:                       
+  .cfi_startproc
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+  subq  $16, %rsp
+  movl  $0, -4(%rbp)
+  callq __Z3foov
+  xorl  %eax, %eax
+  addq  $16, %rsp
+  popq  %rbp
+  retq
+  .cfi_endproc
+
+.subsections_via_symbols