[GlobalISel][IRTranslator] Use preferred alignment when creating frame indices.

[aemerson]

Summary

By using the the preferred alignment when possible, this can allow local stack slot allocation to avoid materializing a base register for stack slots.

[arsenm]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

263

I think the answer is no. You could have a separate pass up-align allocas, and in practice frontends emit them with the preferred alignment to begin with.

For AMDGPU there is no benefit to aligning anything on the stack over 4, and anything more just wastes space. The ABI still requires the higher alignment though, so I would like to have a pass to under align allocas when the alignment isn't visible. I've also long wanted to allow the datalayout to specify a preferred alignment lower than the ABI, but this isn't allowed (D29810 partially addressed this but never went anywhere)

[aemerson]

Guard this with a TLI check. I think a separate pass to do this is overkill.

[arsenm]

llvm/include/llvm/CodeGen/TargetLowering.h

535 ↗

(On Diff #345734)

I really don't like this hook. We have way too many super specific hacky hooks like this. Is this really a concern? Why isn't the frontend setting the preferred alignment to begin with?

[aemerson]

llvm/include/llvm/CodeGen/TargetLowering.h

535 ↗

(On Diff #345734)

in practice frontends emit them with the preferred alignment to begin with.

I can't find where this is happening. I'm too familiar with clang but if I grep for 'getPrefTypeAlign' I only see a few places where it's called there, which look specific to some cases, not general allocas.

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

int IRTranslator::getOrCreateFrameIndex(const AllocaInst &AI) {
 
   uint64_t ElementSize = DL->getTypeAllocSize(AI.getAllocatedType());
   uint64_t Size =
       ElementSize * cast<ConstantInt>(AI.getArraySize())->getZExtValue();
 
   // Always allocate at least one byte.
   Size = std::max<uint64_t>(Size, 1u);
 
+  const TargetFrameLowering *TFI = MF->getSubtarget().getFrameLowering();
+  const Align StackAlign = TFI->getStackAlign();
+  Align FIAlignment = AI.getAlign();
+  // The following code mimics the alignment optimizations done in
+  // FunctionLoweringInfo for SelectionDAG.
+  Type *Ty = AI.getAllocatedType();
+  Align TyPrefAlign = MF->getDataLayout().getPrefTypeAlign(Ty);
+  // The "specified" alignment is the alignment written on the alloca,
+  // or the preferred alignment of the type if none is specified.
+  Align SpecifiedAlign = AI.getAlign();
+
+  // If the preferred alignment of the type is higher than the specified
+  // alignment of the alloca, promote the alignment, as long as it doesn't
+  // require realigning the stack.
+  //
+  // FIXME: Do we really want to second-guess the IR in isel?
+  Align Alignment = std::max(std::min(TyPrefAlign, StackAlign), SpecifiedAlign);
+
+  // Static allocas can be folded into the initial stack frame
+  // adjustment. For targets that don't realign the stack, don't
+  // do this if there is an extra alignment requirement.
+  if (TFI->isStackRealignable() || (Alignment <= StackAlign))
+    FIAlignment = Alignment;
+
   int &FI = FrameIndices[&AI];
-  FI = MF->getFrameInfo().CreateStackObject(Size, AI.getAlign(), false, &AI);
+  FI = MF->getFrameInfo().CreateStackObject(Size, FIAlignment, false, &AI);
   return FI;
 }
 
 Align IRTranslator::getMemOpAlign(const Instruction &I) {
   if (const StoreInst *SI = dyn_cast<StoreInst>(&I))
     return SI->getAlign();
   if (const LoadInst *LI = dyn_cast<LoadInst>(&I))
     return LI->getAlign();

llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

 }
 
 ; Tests for alloca
 ; CHECK-LABEL: name: allocai64
 ; CHECK: stack:
 ; CHECK-NEXT:   - { id: 0, name: ptr1, type: default, offset: 0, size: 8, alignment: 8,
 ; CHECK-NEXT:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
 ; CHECK-NEXT: debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
-; CHECK-NEXT:   - { id: 1, name: ptr2, type: default, offset: 0, size: 8, alignment: 1,
+; CHECK-NEXT:   - { id: 1, name: ptr2, type: default, offset: 0, size: 8, alignment: 8,
 ; CHECK-NEXT:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
 ; CHECK-NEXT:       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 ; CHECK-NEXT:   - { id: 2, name: ptr3, type: default, offset: 0, size: 128, alignment: 8,
 ; CHECK-NEXT:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
 ; CHECK-NEXT:       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 ; CHECK-NEXT:   - { id: 3, name: ptr4, type: default, offset: 0, size: 1, alignment: 8,
 ; CHECK: %{{[0-9]+}}:_(p0) = G_FRAME_INDEX %stack.0.ptr1
 ; CHECK: %{{[0-9]+}}:_(p0) = G_FRAME_INDEX %stack.1.ptr2
 ; CHECK: %{{[0-9]+}}:_(p0) = G_FRAME_INDEX %stack.2.ptr3
 ; CHECK: %{{[0-9]+}}:_(p0) = G_FRAME_INDEX %stack.3.ptr4
 define void @allocai64() {
   %ptr1 = alloca i64
   %ptr2 = alloca i64, align 1
   %ptr3 = alloca i64, i32 16
   %ptr4 = alloca [0 x i64]
   ret void
 }
 
+define void @alloca_increase_alignment()  {
+; The alignment of the frame index should be 8 here since it's the preferred alignment.
+; CHECK: stack:
+; CHECK:   - { id: 0, name: ai, type: default, offset: 0, size: 8, alignment: 8,
+; CHECK:       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+; CHECK:       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  %ai = alloca i8*, align 4
+  ret void
+}
+
+
 ; Tests for br.
 ; CHECK-LABEL: name: uncondbr
 ; CHECK: body:
 ;
 ; ABI/constant lowering and IR-level entry basic block.
 ; CHECK: bb.{{[0-9]+}}.{{[a-zA-Z0-9.]+}}:
 ;
 ; Make sure we have one successor and only one.