[Alignment][NFC] Convert MachineIRBuilder::buildDynStackAlloc to Align

[gchatelet]

Summary

The change in IRTranslator is not trivial but is NFC as far as I can tell.

This is patch is part of a series to introduce an Alignment type.
See this thread for context: http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790

[gchatelet]

rebase

[courbet]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

1889

I think using a ternary would be more readable.

1893

I'd like to get a knowledgeable pair of eye to look at this. We used to align anything smaller than the stack size to 1, now we align to max(AI.align, preftypealign). This sounds OK, but let's be on the safe side.

[gchatelet]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

1893

@aemerson what do you think?

[gchatelet]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

1893

AFAICT it is not clear what it means use G_DYN_STACKALLOC with an alignment of 0
https://llvm.org/docs/GlobalISel/GenericOpcode.html#g-dyn-stackalloc

@aemerson can you help me understand what's going on here? Or add relevant people to the thread?

[aemerson]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

1893

An alignment of 0 I believe is used to mean there is no alignment requirement at all.

[gchatelet]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

1893

Ok I dug into it and 0 or 1 have the same meaning.
https://reviews.llvm.org/D77372

For code simplicity, I'll use 1 here.

[gchatelet]

• Address comments and rebase

[courbet]

llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h

381

What about taking an alignment now that we're clarified the meaning of 0 ? There's only one caller now and no alignment does not have any particular meaning compared to 1.

[gchatelet]

• Using Align instead of MaybeAlign

llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h

public:
   /// Build and insert \p Res = G_DYN_STACKALLOC \p Size, \p Align
   ///
   /// G_DYN_STACKALLOC does a dynamic stack allocation and writes the address of
   /// the allocated memory into \p Res.
   /// \pre setBasicBlock or setMI must have been called.
   /// \pre \p Res must be a generic virtual register with pointer type.
   ///
   /// \return a MachineInstrBuilder for the newly created instruction.
   MachineInstrBuilder buildDynStackAlloc(const DstOp &Res, const SrcOp &Size,
-                                         unsigned Align);
+                                         Align Alignment);
+
+  LLVM_ATTRIBUTE_DEPRECATED(inline MachineInstrBuilder buildDynStackAlloc(
+                                const DstOp &Res, const SrcOp &Size,
+                                unsigned Align),
+                            "Use the version that takes MaybeAlign instead") {
+    return buildDynStackAlloc(Res, Size, assumeAligned(Align));
+  }
 
   /// Build and insert \p Res = G_FRAME_INDEX \p Idx
   ///
   /// G_FRAME_INDEX materializes the address of an alloca value or other
   /// stack-based object.
   ///
   /// \pre setBasicBlock or setMI must have been called.
   /// \pre \p Res must be a generic virtual register with pointer type.

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

bool IRTranslator::translateAlloca(const User &U,
   }
 
   // FIXME: support stack probing for Windows.
   if (MF->getTarget().getTargetTriple().isOSWindows())
     return false;
 
   // Now we're in the harder dynamic case.
   Register NumElts = getOrCreateVReg(*AI.getArraySize());
-
   Type *IntPtrIRTy = DL->getIntPtrType(AI.getType());
   LLT IntPtrTy = getLLTForType(*IntPtrIRTy, *DL);
   if (MRI->getType(NumElts) != IntPtrTy) {
     Register ExtElts = MRI->createGenericVirtualRegister(IntPtrTy);
     MIRBuilder.buildZExtOrTrunc(ExtElts, NumElts);
     NumElts = ExtElts;
   }
 
   Type *Ty = AI.getAllocatedType();
 
   Register AllocSize = MRI->createGenericVirtualRegister(IntPtrTy);
   Register TySize =
       getOrCreateVReg(*ConstantInt::get(IntPtrIRTy, DL->getTypeAllocSize(Ty)));
   MIRBuilder.buildMul(AllocSize, NumElts, TySize);
 
   // Round the size of the allocation up to the stack alignment size
   // by add SA-1 to the size. This doesn't overflow because we're computing
   // an address inside an alloca.
-  unsigned StackAlign =
-      MF->getSubtarget().getFrameLowering()->getStackAlignment();
-  auto SAMinusOne = MIRBuilder.buildConstant(IntPtrTy, StackAlign - 1);
+  Align StackAlign = MF->getSubtarget().getFrameLowering()->getStackAlign();
+  auto SAMinusOne = MIRBuilder.buildConstant(IntPtrTy, StackAlign.value() - 1);
   auto AllocAdd = MIRBuilder.buildAdd(IntPtrTy, AllocSize, SAMinusOne,
                                       MachineInstr::NoUWrap);
   auto AlignCst =
-      MIRBuilder.buildConstant(IntPtrTy, ~(uint64_t)(StackAlign - 1));
+      MIRBuilder.buildConstant(IntPtrTy, ~(uint64_t)(StackAlign.value() - 1));
   auto AlignedAlloc = MIRBuilder.buildAnd(IntPtrTy, AllocAdd, AlignCst);
 
-  unsigned Align =
-      std::max((unsigned)DL->getPrefTypeAlignment(Ty), AI.getAlignment());
-  if (Align <= StackAlign)
-    Align = 0;
-  MIRBuilder.buildDynStackAlloc(getOrCreateVReg(AI), AlignedAlloc, Align);
+  Align Alignment = max(AI.getAlign(), DL->getPrefTypeAlign(Ty));
+  if (Alignment <= StackAlign)
+    Alignment = Align(1);
+  MIRBuilder.buildDynStackAlloc(getOrCreateVReg(AI), AlignedAlloc, Alignment);
 
-  MF->getFrameInfo().CreateVariableSizedObject(Align ? Align : 1, &AI);
+  MF->getFrameInfo().CreateVariableSizedObject(Alignment, &AI);
   assert(MF->getFrameInfo().hasVarSizedObjects());
   return true;
 }
 
 bool IRTranslator::translateVAArg(const User &U, MachineIRBuilder &MIRBuilder) {
   // FIXME: We may need more info about the type. Because of how LLT works,
   // we're completely discarding the i64/double distinction here (amongst
   // others). Fortunately the ABIs I know of where that matters don't use va_arg

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

assert(cast<DILabel>(Label)->isValidLocationForIntrinsic(State.DL) &&
          "Expected inlined-at fields to agree");
   auto MIB = buildInstr(TargetOpcode::DBG_LABEL);
 
   return MIB.addMetadata(Label);
 }
 
 MachineInstrBuilder MachineIRBuilder::buildDynStackAlloc(const DstOp &Res,
                                                          const SrcOp &Size,
-                                                         unsigned Align) {
+                                                         Align Alignment) {
   assert(Res.getLLTTy(*getMRI()).isPointer() && "expected ptr dst type");
   auto MIB = buildInstr(TargetOpcode::G_DYN_STACKALLOC);
   Res.addDefToMIB(*getMRI(), MIB);
   Size.addSrcToMIB(MIB);
-  MIB.addImm(Align);
+  MIB.addImm(Alignment.value());
   return MIB;
 }
 
 MachineInstrBuilder MachineIRBuilder::buildFrameIndex(const DstOp &Res,
                                                       int Idx) {
   assert(Res.getLLTTy(*getMRI()).isPointer() && "invalid operand type");
   auto MIB = buildInstr(TargetOpcode::G_FRAME_INDEX);
   Res.addDefToMIB(*getMRI(), MIB);

llvm/test/CodeGen/AArch64/GlobalISel/dynamic-alloca.ll

 ; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 ; RUN: llc -mtriple=aarch64 -global-isel %s -o - -stop-after=irtranslator | FileCheck %s
 
 define i8* @test_simple_alloca(i32 %numelts) {
   ; CHECK-LABEL: name: test_simple_alloca
   ; CHECK: bb.1 (%ir-block.0):
   ; CHECK:   liveins: $w0
   ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
   ; CHECK:   [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 1
   ; CHECK:   [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[COPY]](s32)
   ; CHECK:   [[MUL:%[0-9]+]]:_(s64) = G_MUL [[ZEXT]], [[C]]
   ; CHECK:   [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 15
   ; CHECK:   [[ADD:%[0-9]+]]:_(s64) = nuw G_ADD [[MUL]], [[C1]]
   ; CHECK:   [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 -16
   ; CHECK:   [[AND:%[0-9]+]]:_(s64) = G_AND [[ADD]], [[C2]]
-  ; CHECK:   [[DYN_STACKALLOC:%[0-9]+]]:_(p0) = G_DYN_STACKALLOC [[AND]](s64), 0
+  ; CHECK:   [[DYN_STACKALLOC:%[0-9]+]]:_(p0) = G_DYN_STACKALLOC [[AND]](s64), 1
   ; CHECK:   $x0 = COPY [[DYN_STACKALLOC]](p0)
   ; CHECK:   RET_ReallyLR implicit $x0
   %addr = alloca i8, i32 %numelts
   ret i8* %addr
 }
 
 define i8* @test_aligned_alloca(i32 %numelts) {
   ; CHECK-LABEL: name: test_aligned_alloca
define i128* @test_natural_alloca(i32 %numelts) {
   ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
   ; CHECK:   [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 16
   ; CHECK:   [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[COPY]](s32)
   ; CHECK:   [[MUL:%[0-9]+]]:_(s64) = G_MUL [[ZEXT]], [[C]]
   ; CHECK:   [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 15
   ; CHECK:   [[ADD:%[0-9]+]]:_(s64) = nuw G_ADD [[MUL]], [[C1]]
   ; CHECK:   [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 -16
   ; CHECK:   [[AND:%[0-9]+]]:_(s64) = G_AND [[ADD]], [[C2]]
-  ; CHECK:   [[DYN_STACKALLOC:%[0-9]+]]:_(p0) = G_DYN_STACKALLOC [[AND]](s64), 0
+  ; CHECK:   [[DYN_STACKALLOC:%[0-9]+]]:_(p0) = G_DYN_STACKALLOC [[AND]](s64), 1
   ; CHECK:   $x0 = COPY [[DYN_STACKALLOC]](p0)
   ; CHECK:   RET_ReallyLR implicit $x0
   %addr = alloca i128, i32 %numelts
   ret i128* %addr
 }