[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 ↗

(On Diff #529965)

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 ↗

(On Diff #529965)

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 ↗

(On Diff #529965)

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 ↗

(On Diff #529965)

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

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

[jyknight]

llvm/lib/MC/MCAsmBackend.cpp

125

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

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

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/MC/MCAsmBackend.cpp

bool MCAsmBackend::fixupNeedsRelaxationAdvanced(
     const MCRelaxableFragment *DF, const MCAsmLayout &Layout,
     const bool WasForced) const {
   if (!Resolved)
     return true;
   return fixupNeedsRelaxation(Fixup, Value, DF, Layout);
 }
 
 bool MCAsmBackend::isDarwinCanonicalPersonality(const MCSymbol *Sym) const {
-  if (Sym && Sym->isMachO()) {
-    StringRef name = Sym->getName();
-    // XXX: We intentionally leave out "___gcc_personality_v0" because, despite
-    // being system-defined like these two, it is not very commonly-used.
-    // Reserving an empty slot for it seems silly.
-    return name == "___gxx_personality_v0" || name == "___objc_personality_v0";
-  }
-  return false;
-}
+  // Consider a NULL personality (ie., no personality encoding) to be canonical
+  // because it's always at 0.
+  if (!Sym)
+    return true;
+
+  if (!Sym->isMachO())
+    llvm_unreachable("Expected MachO symbols only");
+

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