[llvm] r182237 - PR15868 fix.

Stepan Dyatkovskiy stpworld at narod.ru
Mon May 20 01:01:34 PDT 2013


Author: dyatkovskiy
Date: Mon May 20 03:01:34 2013
New Revision: 182237

URL: http://llvm.org/viewvc/llvm-project?rev=182237&view=rev
Log:
PR15868 fix.

Introduction:
In case when stack alignment is 8 and GPRs parameter part size is not N*8:
we add padding to GPRs part, so part's last byte must be recovered at
address K*8-1.
We need to do it, since remained (stack) part of parameter starts from
address K*8, and we need to "attach" "GPRs head" without gaps to it:

Stack:
|---- 8 bytes block ----| |---- 8 bytes block ----| |---- 8 bytes...
[ [padding] [GPRs head] ] [ ------ Tail passed via stack  ------ ...

FIX:
Note, once we added padding we need to correct *all* Arg offsets that are going
after padded one. That's why we need this fix: Arg offsets were never corrected
before this patch. See new test-cases included in patch.

We also don't need to insert padding for byval parameters that are stored in GPRs
only. We need pad only last byval parameter and only in case it outsides GPRs
and stack alignment = 8.
Though, stack area, allocated for recovered byval params, must satisfy
"Size mod 8 = 0" restriction.

This patch reduces stack usage for some cases:
We can reduce ArgRegsSaveArea since inner N*4 bytes sized byval params my be
"packed" with alignment 4 in some cases.


Added:
    llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding.ll
    llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding2.ll
Modified:
    llvm/trunk/lib/Target/ARM/ARMFrameLowering.cpp
    llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
    llvm/trunk/lib/Target/ARM/ARMISelLowering.h
    llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h
    llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp
    llvm/trunk/test/CodeGen/ARM/2013-04-05-Small-ByVal-Structs-PR15293.ll

Modified: llvm/trunk/lib/Target/ARM/ARMFrameLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMFrameLowering.cpp?rev=182237&r1=182236&r2=182237&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMFrameLowering.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMFrameLowering.cpp Mon May 20 03:01:34 2013
@@ -141,7 +141,8 @@ void ARMFrameLowering::emitPrologue(Mach
   assert(!AFI->isThumb1OnlyFunction() &&
          "This emitPrologue does not support Thumb1!");
   bool isARM = !AFI->isThumbFunction();
-  unsigned ArgRegsSaveSize = AFI->getArgRegsSaveSize();
+  unsigned Align = MF.getTarget().getFrameLowering()->getStackAlignment();
+  unsigned ArgRegsSaveSize = AFI->getArgRegsSaveSize(Align);
   unsigned NumBytes = MFI->getStackSize();
   const std::vector<CalleeSavedInfo> &CSI = MFI->getCalleeSavedInfo();
   DebugLoc dl = MBBI != MBB.end() ? MBBI->getDebugLoc() : DebugLoc();
@@ -357,7 +358,8 @@ void ARMFrameLowering::emitEpilogue(Mach
          "This emitEpilogue does not support Thumb1!");
   bool isARM = !AFI->isThumbFunction();
 
-  unsigned ArgRegsSaveSize = AFI->getArgRegsSaveSize();
+  unsigned Align = MF.getTarget().getFrameLowering()->getStackAlignment();
+  unsigned ArgRegsSaveSize = AFI->getArgRegsSaveSize(Align);
   int NumBytes = (int)MFI->getStackSize();
   unsigned FramePtr = RegInfo->getFrameRegister(MF);
 

Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=182237&r1=182236&r2=182237&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Mon May 20 03:01:34 2013
@@ -2630,6 +2630,7 @@ ARMTargetLowering::GetF64FormalArgument(
 void
 ARMTargetLowering::computeRegArea(CCState &CCInfo, MachineFunction &MF,
                                   unsigned InRegsParamRecordIdx,
+                                  unsigned ArgSize,
                                   unsigned &ArgRegsSize,
                                   unsigned &ArgRegsSaveSize)
   const {
@@ -2648,7 +2649,29 @@ ARMTargetLowering::computeRegArea(CCStat
 
   unsigned Align = MF.getTarget().getFrameLowering()->getStackAlignment();
   ArgRegsSize = NumGPRs * 4;
-  ArgRegsSaveSize = (ArgRegsSize + Align - 1) & ~(Align - 1);
+
+  // If parameter is split between stack and GPRs...
+  if (NumGPRs && Align == 8 &&
+      (ArgRegsSize < ArgSize ||
+        InRegsParamRecordIdx >= CCInfo.getInRegsParamsCount())) {
+    // Add padding for part of param recovered from GPRs, so
+    // its last byte must be at address K*8 - 1.
+    // We need to do it, since remained (stack) part of parameter has
+    // stack alignment, and we need to "attach" "GPRs head" without gaps
+    // to it:
+    // Stack:
+    // |---- 8 bytes block ----| |---- 8 bytes block ----| |---- 8 bytes...
+    // [ [padding] [GPRs head] ] [        Tail passed via stack       ....
+    //
+    ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
+    unsigned Padding =
+        ((ArgRegsSize + AFI->getArgRegsSaveSize() + Align - 1) & ~(Align-1)) -
+        (ArgRegsSize + AFI->getArgRegsSaveSize());
+    ArgRegsSaveSize = ArgRegsSize + Padding;
+  } else
+    // We don't need to extend regs save size for byval parameters if they
+    // are passed via GPRs only.
+    ArgRegsSaveSize = ArgRegsSize;
 }
 
 // The remaining GPRs hold either the beginning of variable-argument
@@ -2666,6 +2689,7 @@ ARMTargetLowering::StoreByValRegs(CCStat
                                   unsigned InRegsParamRecordIdx,
                                   unsigned OffsetFromOrigArg,
                                   unsigned ArgOffset,
+                                  unsigned ArgSize,
                                   bool ForceMutable) const {
 
   // Currently, two use-cases possible:
@@ -2695,7 +2719,8 @@ ARMTargetLowering::StoreByValRegs(CCStat
   }
 
   unsigned ArgRegsSize, ArgRegsSaveSize;
-  computeRegArea(CCInfo, MF, InRegsParamRecordIdx, ArgRegsSize, ArgRegsSaveSize);
+  computeRegArea(CCInfo, MF, InRegsParamRecordIdx, ArgSize,
+                 ArgRegsSize, ArgRegsSaveSize);
 
   // Store any by-val regs to their spots on the stack so that they may be
   // loaded by deferencing the result of formal parameter pointer or va_next.
@@ -2703,9 +2728,17 @@ ARMTargetLowering::StoreByValRegs(CCStat
   // was initialized, it can't be initialized again.
   if (ArgRegsSaveSize) {
 
+    unsigned Padding = ArgRegsSaveSize - ArgRegsSize;
+
+    if (Padding) {
+      assert(AFI->getStoredByValParamsPadding() == 0 &&
+             "The only parameter may be padded.");
+      AFI->setStoredByValParamsPadding(Padding);
+    }
+
     int FrameIndex = MFI->CreateFixedObject(
                       ArgRegsSaveSize,
-                      ArgOffset + ArgRegsSaveSize - ArgRegsSize,
+                      Padding + ArgOffset,
                       false);
     SDValue FIN = DAG.getFrameIndex(FrameIndex, getPointerTy());
 
@@ -2737,7 +2770,8 @@ ARMTargetLowering::StoreByValRegs(CCStat
     return FrameIndex;
   } else
     // This will point to the next argument passed via stack.
-    return MFI->CreateFixedObject(4, ArgOffset, !ForceMutable);
+    return MFI->CreateFixedObject(
+        4, AFI->getStoredByValParamsPadding() + ArgOffset, !ForceMutable);
 }
 
 // Setup stack frame, the va_list pointer will start from.
@@ -2756,7 +2790,7 @@ ARMTargetLowering::VarArgStyleRegisters(
   // argument passed via stack.
   int FrameIndex =
     StoreByValRegs(CCInfo, DAG, dl, Chain, 0, CCInfo.getInRegsParamsCount(),
-                   0, ArgOffset, ForceMutable);
+                   0, ArgOffset, 0, ForceMutable);
 
   AFI->setVarArgsFrameIndex(FrameIndex);
 }
@@ -2896,12 +2930,15 @@ ARMTargetLowering::LowerFormalArguments(
                 CurByValIndex,
                 Ins[VA.getValNo()].PartOffset,
                 VA.getLocMemOffset(),
+                Flags.getByValSize(),
                 true /*force mutable frames*/);
             InVals.push_back(DAG.getFrameIndex(FrameIndex, getPointerTy()));
             CCInfo.nextInRegsParam();
           } else {
+            unsigned FIOffset = VA.getLocMemOffset() +
+                                AFI->getStoredByValParamsPadding();
             int FI = MFI->CreateFixedObject(VA.getLocVT().getSizeInBits()/8,
-                                            VA.getLocMemOffset(), true);
+                                            FIOffset, true);
 
             // Create load nodes to retrieve arguments from the stack.
             SDValue FIN = DAG.getFrameIndex(FI, getPointerTy());

Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.h?rev=182237&r1=182236&r2=182237&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMISelLowering.h (original)
+++ llvm/trunk/lib/Target/ARM/ARMISelLowering.h Mon May 20 03:01:34 2013
@@ -480,6 +480,7 @@ namespace llvm {
                        unsigned InRegsParamRecordIdx,
                        unsigned OffsetFromOrigArg,
                        unsigned ArgOffset,
+                       unsigned ArgSize,
                        bool ForceMutable) const;
 
     void VarArgStyleRegisters(CCState &CCInfo, SelectionDAG &DAG,
@@ -489,6 +490,7 @@ namespace llvm {
 
     void computeRegArea(CCState &CCInfo, MachineFunction &MF,
                         unsigned InRegsParamRecordIdx,
+                        unsigned ArgSize,
                         unsigned &ArgRegsSize,
                         unsigned &ArgRegsSaveSize) const;
 

Modified: llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h?rev=182237&r1=182236&r2=182237&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h (original)
+++ llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h Mon May 20 03:01:34 2013
@@ -36,6 +36,13 @@ class ARMFunctionInfo : public MachineFu
   /// 'isThumb'.
   bool hasThumb2;
 
+  /// StByValParamsPadding - For parameter that is split between
+  /// GPRs and memory; while recovering GPRs part, when
+  /// StackAlignment == 8, and GPRs-part-size mod 8 != 0,
+  /// we need to insert gap before parameter start address. It allows to
+  /// "attach" GPR-part to the part that was passed via stack.
+  unsigned StByValParamsPadding;
+
   /// VarArgsRegSaveSize - Size of the register save area for vararg functions.
   ///
   unsigned ArgRegsSaveSize;
@@ -129,6 +136,7 @@ public:
   explicit ARMFunctionInfo(MachineFunction &MF) :
     isThumb(MF.getTarget().getSubtarget<ARMSubtarget>().isThumb()),
     hasThumb2(MF.getTarget().getSubtarget<ARMSubtarget>().hasThumb2()),
+    StByValParamsPadding(0),
     ArgRegsSaveSize(0), HasStackFrame(false), RestoreSPFromFP(false),
     LRSpilledForFarJump(false),
     FramePtrSpillOffset(0), GPRCS1Offset(0), GPRCS2Offset(0), DPRCSOffset(0),
@@ -141,7 +149,14 @@ public:
   bool isThumb1OnlyFunction() const { return isThumb && !hasThumb2; }
   bool isThumb2Function() const { return isThumb && hasThumb2; }
 
-  unsigned getArgRegsSaveSize() const { return ArgRegsSaveSize; }
+  unsigned getStoredByValParamsPadding() const { return StByValParamsPadding; }
+  void setStoredByValParamsPadding(unsigned p) { StByValParamsPadding = p; }
+
+  unsigned getArgRegsSaveSize(unsigned Align = 0) const {
+    if (!Align)
+      return ArgRegsSaveSize;
+    return (ArgRegsSaveSize + Align - 1) & ~(Align - 1);
+  }
   void setArgRegsSaveSize(unsigned s) { ArgRegsSaveSize = s; }
 
   bool hasStackFrame() const { return HasStackFrame; }

Modified: llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp?rev=182237&r1=182236&r2=182237&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp (original)
+++ llvm/trunk/lib/Target/ARM/Thumb1FrameLowering.cpp Mon May 20 03:01:34 2013
@@ -88,7 +88,8 @@ void Thumb1FrameLowering::emitPrologue(M
   const Thumb1InstrInfo &TII =
     *static_cast<const Thumb1InstrInfo*>(MF.getTarget().getInstrInfo());
 
-  unsigned ArgRegsSaveSize = AFI->getArgRegsSaveSize();
+  unsigned Align = MF.getTarget().getFrameLowering()->getStackAlignment();
+  unsigned ArgRegsSaveSize = AFI->getArgRegsSaveSize(Align);
   unsigned NumBytes = MFI->getStackSize();
   const std::vector<CalleeSavedInfo> &CSI = MFI->getCalleeSavedInfo();
   DebugLoc dl = MBBI != MBB.end() ? MBBI->getDebugLoc() : DebugLoc();
@@ -249,7 +250,8 @@ void Thumb1FrameLowering::emitEpilogue(M
   const Thumb1InstrInfo &TII =
     *static_cast<const Thumb1InstrInfo*>(MF.getTarget().getInstrInfo());
 
-  unsigned ArgRegsSaveSize = AFI->getArgRegsSaveSize();
+  unsigned Align = MF.getTarget().getFrameLowering()->getStackAlignment();
+  unsigned ArgRegsSaveSize = AFI->getArgRegsSaveSize(Align);
   int NumBytes = (int)MFI->getStackSize();
   const uint16_t *CSRegs = RegInfo->getCalleeSavedRegs();
   unsigned FramePtr = RegInfo->getFrameRegister(MF);

Modified: llvm/trunk/test/CodeGen/ARM/2013-04-05-Small-ByVal-Structs-PR15293.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2013-04-05-Small-ByVal-Structs-PR15293.ll?rev=182237&r1=182236&r2=182237&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/2013-04-05-Small-ByVal-Structs-PR15293.ll (original)
+++ llvm/trunk/test/CodeGen/ARM/2013-04-05-Small-ByVal-Structs-PR15293.ll Mon May 20 03:01:34 2013
@@ -4,24 +4,24 @@
 ;CHECK: foo:
 ;CHECK: 	sub	sp, sp, #8
 ;CHECK: 	push	{r11, lr}
-;CHECK: 	str	r0, [sp, #12]
-;CHECK: 	add	r0, sp, #12
+;CHECK: 	str	r0, [sp, #8]
+;CHECK: 	add	r0, sp, #8
 ;CHECK: 	bl	fooUseParam
 ;CHECK: 	pop	{r11, lr}
 ;CHECK: 	add	sp, sp, #8
 ;CHECK: 	mov	pc, lr
 
 ;CHECK: foo2:
-;CHECK: 	sub	sp, sp, #16
+;CHECK: 	sub	sp, sp, #8
 ;CHECK: 	push	{r11, lr}
-;CHECK: 	str	r0, [sp, #12]
-;CHECK: 	add	r0, sp, #12
-;CHECK: 	str	r2, [sp, #16]
+;CHECK: 	str	r0, [sp, #8]
+;CHECK: 	add	r0, sp, #8
+;CHECK: 	str	r2, [sp, #12]
 ;CHECK: 	bl	fooUseParam
-;CHECK: 	add	r0, sp, #16
+;CHECK: 	add	r0, sp, #12
 ;CHECK: 	bl	fooUseParam
 ;CHECK: 	pop	{r11, lr}
-;CHECK: 	add	sp, sp, #16
+;CHECK: 	add	sp, sp, #8
 ;CHECK: 	mov	pc, lr
 
 ;CHECK: doFoo:

Added: llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding.ll?rev=182237&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding.ll (added)
+++ llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding.ll Mon May 20 03:01:34 2013
@@ -0,0 +1,31 @@
+;PR15293: ARM codegen ice - expected larger existing stack allocation
+;RUN: llc -mtriple=arm-linux-gnueabihf < %s | FileCheck %s
+
+%struct.S227 = type { [49 x i32], i32 }
+
+define void @check227(
+                      i32 %b,                              
+                      %struct.S227* byval nocapture %arg0,
+                      %struct.S227* %arg1) {
+; b --> R0
+; arg0 --> [R1, R2, R3, SP+0 .. SP+188)
+; arg1 --> SP+188
+
+entry:
+
+;CHECK:  sub   sp, sp, #16
+;CHECK:  push  {r11, lr}
+;CHECK:  add   r0, sp, #12
+;CHECK:  stm   r0, {r1, r2, r3}
+;CHECK:  ldr   r0, [sp, #212]
+;CHECK:  bl    useInt
+;CHECK:  pop   {r11, lr}
+;CHECK:  add   sp, sp, #16
+
+  %0 = ptrtoint %struct.S227* %arg1 to i32
+  tail call void @useInt(i32 %0)
+  ret void
+}
+
+declare void @useInt(i32)
+

Added: llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding2.ll?rev=182237&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding2.ll (added)
+++ llvm/trunk/test/CodeGen/ARM/2013-05-13-AAPCS-byval-padding2.ll Mon May 20 03:01:34 2013
@@ -0,0 +1,25 @@
+;PR15293: ARM codegen ice - expected larger existing stack allocation
+;RUN: llc -mtriple=arm-linux-gnueabihf < %s | FileCheck %s
+
+%struct4bytes = type { i32 }
+%struct20bytes = type { i32, i32, i32, i32, i32 }
+
+define void @foo(%struct4bytes* byval %p0, ; --> R0
+                 %struct20bytes* byval %p1 ; --> R1,R2,R3, [SP+0 .. SP+8)
+) {
+;CHECK:  sub  sp, sp, #16
+;CHECK:  push  {r11, lr}
+;CHECK:  add  r11, sp, #8
+;CHECK:  stm  r11, {r0, r1, r2, r3}
+;CHECK:  add  r0, sp, #12
+;CHECK:  bl  useInt
+;CHECK:  pop  {r11, lr}
+;CHECK:  add  sp, sp, #16
+
+  %1 = ptrtoint %struct20bytes* %p1 to i32
+  tail call void @useInt(i32 %1)
+  ret void
+}
+
+declare void @useInt(i32)
+





More information about the llvm-commits mailing list