Fix unwind information for floating point registers

[Jackson]

Summary

Fixes the unwind information generated for floating-point registers. Previously, all padding registers were assumed to be four bytes wide. Now, the width of the register is used to specify the amount of padding.

[olista01]

lib/Target/ARM/ARMAsmPrinter.cpp

1107–1109

We already get the MachineFunction and TargetRegisterInfo at the top of this function, and the MachineRegisterInfo should be moved up there to keep these together.

test/CodeGen/ARM/unwind-fp.ll

2

This test can be simplified a lot, to something like this:

target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
target triple = "armv7-arm-none-eabi"

define void @foo() minsize {
entry:
  %a = alloca i32, align 4
  call void asm sideeffect "", "r,~{d8}"(i32* %a)
  ret void
}

Also, it would be clearer to put the CHECK line inside the IR for the function being tested, and to also check the other frame directives and the actual frame setup instructions.

[Jackson]

Thanks for the feedback. I have vastly simplified the testcase as suggested and deduplicated and lifted the definitions of {Target,Machine}RegisterInformation and MachineFunction out of the loop.

[Jackson]

Thanks for the review. Since I don't have commit rights, could this be committed on my behalf?

lib/Target/ARM/ARMAsmPrinter.cpp

 
 void ARMAsmPrinter::EmitUnwindingInstruction(const MachineInstr *MI) {
   assert(MI->getFlag(MachineInstr::FrameSetup) &&
       "Only instruction which are involved into frame setup code are allowed");
 
   MCTargetStreamer &TS = *OutStreamer->getTargetStreamer();
   ARMTargetStreamer &ATS = static_cast<ARMTargetStreamer &>(TS);
   const MachineFunction &MF = *MI->getParent()->getParent();
-  const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
+  const TargetRegisterInfo *TargetRegInfo =
+    MF.getSubtarget().getRegisterInfo();
+  const MachineRegisterInfo &MachineRegInfo = MF.getRegInfo();
   const ARMFunctionInfo &AFI = *MF.getInfo<ARMFunctionInfo>();
 
-  unsigned FramePtr = RegInfo->getFrameRegister(MF);
+  unsigned FramePtr = TargetRegInfo->getFrameRegister(MF);
   unsigned Opc = MI->getOpcode();
   unsigned SrcReg, DstReg;
 
   if (Opc == ARM::tPUSH || Opc == ARM::tLDRpci) {
     // Two special cases:
     // 1) tPUSH does not have src/dst regs.
     // 2) for Thumb1 code we sometimes materialize the constant via constpool
     // load. Yes, this is pretty fragile, but for now I don't see better
case ARM::VSTMDDB_UPD:
           continue;
         // Registers, pushed as a part of folding an SP update into the
         // push instruction are marked as undef and should not be
         // restored when unwinding, because the function can modify the
         // corresponding stack slots.
         if (MO.isUndef()) {
           assert(RegList.empty() &&
                  "Pad registers must come before restored ones");
-          Pad += 4;
+          unsigned Width =
+            TargetRegInfo->getRegSizeInBits(MO.getReg(), MachineRegInfo) / 8;
+          Pad += Width;
           continue;
         }
         RegList.push_back(MO.getReg());
       }
       break;
     case ARM::STR_PRE_IMM:
     case ARM::STR_PRE_REG:
     case ARM::t2STR_PRE:

test/CodeGen/ARM/unwind-fp.ll

+; RUN: llc < %s -mtriple=armv7a-arm-none-eabi | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "armv7-arm-none-eabi"
+
+define void @foo() minsize {
+entry:
+  ; CHECK: .vsave {[[SAVE_REG:d[0-9]+]]}
+  ; CHECK-NEXT: .pad #8
+  ; CHECK-NEXT: vpush   {[[PAD_REG:d[0-9]+]], [[SAVE_REG]]}
+  ; CHECK: vpop     {[[PAD_REG]], [[SAVE_REG]]}
+  %a = alloca i32, align 4
+  call void asm sideeffect "", "r,~{d8}"(i32* %a)
+  ret void
+}