[X86AsmBackend] Emit compact unwind for register-sized stacks

[bruno]

Summary

For stack frames on the size of a register in x86, a code size optimization
emits "push rax/eax" instead of "sub" for stack allocation. For example:

foo:

.cfi_startproc

BB#0:

pushq %rax

Ltmp0:

.cfi_def_cfa_offset 16
...
.cfi_endproc

However, according to this code snippet in X86AsmBackend:

If the amount of the stack allocation is the size of a register, then
we "push" the RAX/EAX register onto the stack instead of adjusting the
stack pointer with a SUB instruction. We don't support the push of the
RAX/EAX register with compact unwind. So we check for that situation
// here.
if ((NumDefCFAOffsets == SavedRegIdx + 1 &&

   StackSize - PrevStackSize == 1) ||
  (Instrs.size() == 1 && NumDefCFAOffsets == 1 && StackSize == 2))
return CU::UNWIND_MODE_DWARF;

we cannot use compact unwind in this function since encoding "push rax/eax"
isn't supported. This looks overzealous since we really don't need to
encode rax/eax here, specially because we don't care about %rax content, i.e.,
there's no .cfi_offset %rax, <offset> being used.

It's also overzealous in the case where there are pushes for callee saved
registers followed by a "push rax/eax" instead of "sub", in which we should
also be able to encode the callee saved regs and everything else using compact
unwind.

This patch removes this restriction, if the approach looks sane I'll include
a testcase for the compact unwind part.

[bruno]

Ping!

[t.p.northover]

Hi Bruno,

Like you, I think this ought to just work. The compact unwind format shouldn't care how we subtracted 4/8 from the stack. I think you should go for it (with a test, as you said).

And sorry it took so long to look at this properly.

Tim.

[Eugene.Zelenko]

Looks like patch was not committed.

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

if (HasFP) {
       // Get the encoding of the saved registers when we have a frame pointer.
       uint32_t RegEnc = encodeCompactUnwindRegistersWithFrame();
       if (RegEnc == ~0U) return CU::UNWIND_MODE_DWARF;
 
       CompactUnwindEncoding |= CU::UNWIND_MODE_BP_FRAME;
       CompactUnwindEncoding |= (StackAdjust & 0xFF) << 16;
       CompactUnwindEncoding |= RegEnc & CU::UNWIND_BP_FRAME_REGISTERS;
     } else {
-      // If the amount of the stack allocation is the size of a register, then
-      // we "push" the RAX/EAX register onto the stack instead of adjusting the
-      // stack pointer with a SUB instruction. We don't support the push of the
-      // RAX/EAX register with compact unwind. So we check for that situation
-      // here.
-      if ((NumDefCFAOffsets == SavedRegIdx + 1 &&
-           StackSize - PrevStackSize == 1) ||
-          (Instrs.size() == 1 && NumDefCFAOffsets == 1 && StackSize == 2))
-        return CU::UNWIND_MODE_DWARF;
-
       SubtractInstrIdx += InstrOffset;
       ++StackAdjust;
 
       if ((StackSize & 0xFF) == StackSize) {
         // Frameless stack with a small stack size.
         CompactUnwindEncoding |= CU::UNWIND_MODE_STACK_IMMD;
 
         // Encode the stack size.

llvm/trunk/test/MC/X86/compact-unwind.s

Ltmp14:
   .cfi_offset %rbp, -16
   xorl  %eax, %eax
   addq  $408, %rsp              ## imm = 0x198
   popq  %rbx
   popq  %rbp
   retq
   .cfi_endproc
 
+# Check that a adjustment through a push %rax is the same as a sub.
+
+# CHECK:   Entry at offset 0x40:
+# CHECK-NEXT:     start:                0x2a _testrax
+# CHECK-NEXT:     length:               0x5
+# CHECK-NEXT:     compact encoding:     0x02020000
+  .globl  _testrax
+_testrax:                               ## @testrax
+  .cfi_startproc
+## %bb.0:                               ## %entry
+  pushq %rax
+Ltmp15:
+  .cfi_def_cfa_offset 16
+  xorl  %eax, %eax
+  popq  %rax
+  retq
+  .cfi_endproc
+
   .section  __TEXT,__cstring,cstring_literals
 L_.str:                                 ## @.str
   .asciz  "%d\n"
 
 
 .subsections_via_symbols