[Alignment][NFC] Use Align for TargetCallingConv::OrigAlign

[gchatelet]

Summary

This patch replaces D69249.

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

[courbet]

I see several places that never call setOrigAlign(), e.g. in GlobalISel CallLowering:

ISD::ArgFlagsTy Flags = OrigFlags;
            if (Part == 0) {
              Flags.setSplit();
            } else {
              Flags.setOrigAlign(Align(1));
              if (Part == NumParts - 1)
                Flags.setSplitEnd();
            }
            Args[i].Regs.push_back(Reg);
            Args[i].Flags.push_back(Flags);

How did you infer that this change was safe ?

[gchatelet]

• Make OrigAlign defaults to Align(1)

[gchatelet]

In D82307#2108220, @courbet wrote:

I see several places that never call setOrigAlign(), e.g. in GlobalISel CallLowering:

ISD::ArgFlagsTy Flags = OrigFlags;
            if (Part == 0) {
              Flags.setSplit();
            } else {
              Flags.setOrigAlign(Align(1));
              if (Part == NumParts - 1)
                Flags.setSplitEnd();
            }
            Args[i].Regs.push_back(Reg);
            Args[i].Flags.push_back(Flags);

How did you infer that this change was safe ?

So indeed, running the tests in debug mode revealed a few architectures for which OrigAlign was not set.
Namely codegen for ARM and Mips.

I looked at all the stack traces. They either use the alignment to check on some particular values and make decisions based on them (e.g. which registers to use) or passed down to AllocateStackwhich requires the alignment to be defined.

By switching the default value to Align(1) instead of None, all the tests are passing.
It is not a proof that the change is safe for sure but we're in CodeGen and alignment should be resolved at this point anyways.

[courbet]

OK but this changes the meaning of getOrigAlign(), which some downstream targets might depend on. What about doing this dynamically in getNonZeroOrigAlign() until getOrigAlign() is fully removed ?

[gchatelet]

Address comments

[gchatelet]

In D82307#2113385, @courbet wrote:

OK but this changes the meaning of getOrigAlign(), which some downstream targets might depend on. What about doing this dynamically in getNonZeroOrigAlign() until getOrigAlign() is fully removed ?

I kept the original semantic for getOrigAlign as you suggested.

llvm/include/llvm/CodeGen/TargetCallingConv.h

private:
 
     unsigned PointerAddrSpace; ///< Address space of pointer argument
 
   public:
     ArgFlagsTy()
         : IsZExt(0), IsSExt(0), IsInReg(0), IsSRet(0), IsByVal(0), IsNest(0),
           IsReturned(0), IsSplit(0), IsInAlloca(0), IsPreallocated(0),
           IsSplitEnd(0), IsSwiftSelf(0), IsSwiftError(0), IsCFGuardTarget(0),
-          IsHva(0), IsHvaStart(0), IsSecArgPass(0), ByValAlign(0), OrigAlign(0),
+          IsHva(0), IsHvaStart(0), IsSecArgPass(0), ByValAlign(0), OrigAlign(1),
           IsInConsecutiveRegsLast(0), IsInConsecutiveRegs(0),
           IsCopyElisionCandidate(0), IsPointer(0), ByValSize(0),
           PointerAddrSpace(0) {
       static_assert(sizeof(*this) == 3 * sizeof(unsigned), "flags are too big");
     }
 
     bool isZExt() const { return IsZExt; }
     void setZExt() { IsZExt = 1; }
Align getNonZeroByValAlign() const {
       assert(A && "ByValAlign must be defined");
       return *A;
     }
     void setByValAlign(Align A) {
       ByValAlign = encode(A);
       assert(getNonZeroByValAlign() == A && "bitfield overflow");
     }
 
-    unsigned getOrigAlign() const {
+    LLVM_ATTRIBUTE_DEPRECATED(unsigned getOrigAlign() const,
+                              "Use getNonZeroOrigAlign() instead") {
       MaybeAlign A = decodeMaybeAlign(OrigAlign);
       return A ? A->value() : 0;
     }
+    Align getNonZeroOrigAlign() const {
+      MaybeAlign A = decodeMaybeAlign(OrigAlign);
+      assert(A && "OrigAlign must be defined");
+      return *A;
+    }
     void setOrigAlign(Align A) {
       OrigAlign = encode(A);
-      assert(getOrigAlign() == A.value() && "bitfield overflow");
+      assert(getNonZeroOrigAlign() == A && "bitfield overflow");
     }
 
     unsigned getByValSize() const { return ByValSize; }
     void setByValSize(unsigned S) { ByValSize = S; }
 
     unsigned getPointerAddrSpace() const { return PointerAddrSpace; }
     void setPointerAddrSpace(unsigned AS) { PointerAddrSpace = AS; }
 };

llvm/lib/Target/AArch64/AArch64CallingConvention.cpp

static const MCPhysReg QRegList[] = {AArch64::Q0, AArch64::Q1, AArch64::Q2,
                                      AArch64::Q6, AArch64::Q7};
 
 static bool finishStackBlock(SmallVectorImpl<CCValAssign> &PendingMembers,
                              MVT LocVT, ISD::ArgFlagsTy &ArgFlags,
                              CCState &State, Align SlotAlign) {
   unsigned Size = LocVT.getSizeInBits() / 8;
   const Align StackAlign =
       State.getMachineFunction().getDataLayout().getStackAlignment();
-  const Align OrigAlign(ArgFlags.getOrigAlign());
+  const Align OrigAlign = ArgFlags.getNonZeroOrigAlign();
   const Align Alignment = std::min(OrigAlign, StackAlign);
 
   for (auto &It : PendingMembers) {
     It.convertToMem(State.AllocateStack(Size, std::max(Alignment, SlotAlign)));
     State.addLoc(It);
     SlotAlign = Align(1);
   }
 

llvm/lib/Target/AArch64/AArch64CallingConvention.td

 //=- AArch64CallingConv.td - Calling Conventions for AArch64 -*- tablegen -*-=//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This describes the calling conventions for AArch64 architecture.
 //
 //===----------------------------------------------------------------------===//
 
-/// CCIfAlign - Match of the original alignment of the arg
-class CCIfAlign<string Align, CCAction A> :
-  CCIf<!strconcat("ArgFlags.getOrigAlign() == ", Align), A>;
 /// CCIfBigEndian - Match only if we're in big endian mode.
 class CCIfBigEndian<CCAction A> :
   CCIf<"State.getMachineFunction().getDataLayout().isBigEndian()", A>;
 
 class CCIfILP32<CCAction A> :
   CCIf<"State.getMachineFunction().getDataLayout().getPointerSize() == 4", A>;
 
 

llvm/lib/Target/ARM/ARMCallingConv.cpp

static bool CC_ARM_AAPCS_Custom_Aggregate(unsigned ValNo, MVT ValVT,
   // AAPCS HFAs must have 1-4 elements, all of the same type
   if (PendingMembers.size() > 0)
     assert(PendingMembers[0].getLocVT() == LocVT);
 
   // Add the argument to the list to be allocated once we know the size of the
   // aggregate. Store the type's required alignment as extra info for later: in
   // the [N x i64] case all trace has been removed by the time we actually get
   // to do allocation.
-  PendingMembers.push_back(CCValAssign::getPending(ValNo, ValVT, LocVT, LocInfo,
-                                                   ArgFlags.getOrigAlign()));
+  PendingMembers.push_back(CCValAssign::getPending(
+      ValNo, ValVT, LocVT, LocInfo, ArgFlags.getNonZeroOrigAlign().value()));
 
   if (!ArgFlags.isInConsecutiveRegsLast())
     return true;
 
   // Try to allocate a contiguous block of registers, each of the correct
   // size to hold one member.
   auto &DL = State.getMachineFunction().getDataLayout();
   const Align StackAlign = DL.getStackAlignment();

llvm/lib/Target/ARM/ARMCallingConv.td

 //===-- ARMCallingConv.td - Calling Conventions for ARM ----*- tablegen -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 // This describes the calling conventions for ARM architecture.
 //===----------------------------------------------------------------------===//
 
 /// CCIfAlign - Match of the original alignment of the arg
 class CCIfAlign<string Align, CCAction A>:
-  CCIf<!strconcat("ArgFlags.getOrigAlign() == ", Align), A>;
+  CCIf<!strconcat("ArgFlags.getNonZeroOrigAlign() == ", Align), A>;
 
 //===----------------------------------------------------------------------===//
 // ARM APCS Calling Convention
 //===----------------------------------------------------------------------===//
 let Entry = 1 in
 def CC_ARM_APCS : CallingConv<[
 
   // Handles byval parameters.

 def CC_ARM_AAPCS_Common : CallingConv<[
 
   CCIfType<[i1, i8, i16], CCPromoteToType<i32>>,
 
   // i64/f64 is passed in even pairs of GPRs
   // i64 is 8-aligned i32 here, so we may need to eat R1 as a pad register
   // (and the same is true for f64 if VFP is not enabled)
   CCIfType<[i32], CCIfAlign<"8", CCAssignToRegWithShadow<[R0, R2], [R0, R1]>>>,
-  CCIfType<[i32], CCIf<"ArgFlags.getOrigAlign() != 8",
+  CCIfType<[i32], CCIf<"ArgFlags.getNonZeroOrigAlign() != Align(8)",
                        CCAssignToReg<[R0, R1, R2, R3]>>>,
 
   CCIfType<[i32], CCIfAlign<"8", CCAssignToStackWithShadow<4, 8, [R0, R1, R2, R3]>>>,
   CCIfType<[i32], CCAssignToStackWithShadow<4, 4, [R0, R1, R2, R3]>>,
   CCIfType<[f16, bf16, f32], CCAssignToStackWithShadow<4, 4, [Q0, Q1, Q2, Q3]>>,
   CCIfType<[f64], CCAssignToStackWithShadow<8, 8, [Q0, Q1, Q2, Q3]>>,
   CCIfType<[v2f64], CCIfAlign<"16",
            CCAssignToStackWithShadow<16, 16, [Q0, Q1, Q2, Q3]>>>,

llvm/lib/Target/Mips/MipsISelLowering.cpp

static bool CC_MipsO32(unsigned ValNo, MVT ValVT, MVT LocVT,
 
   unsigned Reg;
 
   // f32 and f64 are allocated in A0, A1, A2, A3 when either of the following
   // is true: function is vararg, argument is 3rd or higher, there is previous
   // argument which is not f32 or f64.
   bool AllocateFloatsInIntReg = State.isVarArg() || ValNo > 1 ||
                                 State.getFirstUnallocated(F32Regs) != ValNo;
-  unsigned OrigAlign = ArgFlags.getOrigAlign();
-  bool isI64 = (ValVT == MVT::i32 && OrigAlign == 8);
+  Align OrigAlign = ArgFlags.getNonZeroOrigAlign();
+  bool isI64 = (ValVT == MVT::i32 && OrigAlign == Align(8));
   bool isVectorFloat = MipsState->WasOriginalArgVectorFloat(ValNo);
 
   // The MIPS vector ABI for floats passes them in a pair of registers
   if (ValVT == MVT::i32 && isVectorFloat) {
     // This is the start of an vector that was scalarized into an unknown number
     // of components. It doesn't matter how many there are. Allocate one of the
     // notional 8 byte aligned registers which map onto the argument stack, and
     // shadow the register lost to alignment requirements.
if (ValVT == MVT::f32) {
       if (Reg2 == Mips::A1 || Reg2 == Mips::A3)
         State.AllocateReg(IntRegs);
       State.AllocateReg(IntRegs);
     }
   } else
     llvm_unreachable("Cannot handle this ValVT.");
 
   if (!Reg) {
-    unsigned Offset =
-        State.AllocateStack(ValVT.getStoreSize(), Align(OrigAlign));
+    unsigned Offset = State.AllocateStack(ValVT.getStoreSize(), OrigAlign);
     State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset, LocVT, LocInfo));
   } else
     State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo));
 
   return false;
 }
 
 static bool CC_MipsO32_FP32(unsigned ValNo, MVT ValVT,

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

static bool CC_RISCVAssign2XLen(unsigned XLen, CCState &State, CCValAssign VA1,
                                 ISD::ArgFlagsTy ArgFlags2) {
   unsigned XLenInBytes = XLen / 8;
   if (Register Reg = State.AllocateReg(ArgGPRs)) {
     // At least one half can be passed via register.
     State.addLoc(CCValAssign::getReg(VA1.getValNo(), VA1.getValVT(), Reg,
                                      VA1.getLocVT(), CCValAssign::Full));
   } else {
     // Both halves must be passed on the stack, with proper alignment.
-    unsigned StackAlign = std::max(XLenInBytes, ArgFlags1.getOrigAlign());
+    Align StackAlign =
+        std::max(Align(XLenInBytes), ArgFlags1.getNonZeroOrigAlign());
     State.addLoc(
         CCValAssign::getMem(VA1.getValNo(), VA1.getValVT(),
-                            State.AllocateStack(XLenInBytes, Align(StackAlign)),
+                            State.AllocateStack(XLenInBytes, StackAlign),
                             VA1.getLocVT(), CCValAssign::Full));
     State.addLoc(CCValAssign::getMem(
         ValNo2, ValVT2, State.AllocateStack(XLenInBytes, Align(XLenInBytes)),
         LocVT2, CCValAssign::Full));
     return false;
   }
 
   if (Register Reg = State.AllocateReg(ArgGPRs)) {
static bool CC_RISCV(const DataLayout &DL, RISCVABI::ABI ABI, unsigned ValNo,
   // If this is a variadic argument, the RISC-V calling convention requires
   // that it is assigned an 'even' or 'aligned' register if it has 8-byte
   // alignment (RV32) or 16-byte alignment (RV64). An aligned register should
   // be used regardless of whether the original argument was split during
   // legalisation or not. The argument will not be passed by registers if the
   // original type is larger than 2*XLEN, so the register alignment rule does
   // not apply.
   unsigned TwoXLenInBytes = (2 * XLen) / 8;
-  if (!IsFixed && ArgFlags.getOrigAlign() == TwoXLenInBytes &&
+  if (!IsFixed && ArgFlags.getNonZeroOrigAlign() == TwoXLenInBytes &&
       DL.getTypeAllocSize(OrigTy) == TwoXLenInBytes) {
     unsigned RegIdx = State.getFirstUnallocated(ArgGPRs);
     // Skip 'odd' register if necessary.
     if (RegIdx != array_lengthof(ArgGPRs) && RegIdx % 2 == 1)
       State.AllocateReg(ArgGPRs);
   }
 
   SmallVectorImpl<CCValAssign> &PendingLocs = State.getPendingLocs();

llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

if (IsVarArg) {
     // compute their offsets and the total amount of buffer space needed.
     for (unsigned I = NumFixedArgs; I < Outs.size(); ++I) {
       const ISD::OutputArg &Out = Outs[I];
       SDValue &Arg = OutVals[I];
       EVT VT = Arg.getValueType();
       assert(VT != MVT::iPTR && "Legalized args should be concrete");
       Type *Ty = VT.getTypeForEVT(*DAG.getContext());
       Align Alignment =
-          std::max(Align(Out.Flags.getOrigAlign()), Layout.getABITypeAlign(Ty));
+          std::max(Out.Flags.getNonZeroOrigAlign(), Layout.getABITypeAlign(Ty));
       unsigned Offset =
           CCInfo.AllocateStack(Layout.getTypeAllocSize(Ty), Alignment);
       CCInfo.addLoc(CCValAssign::getMem(ArgLocs.size(), VT.getSimpleVT(),
                                         Offset, VT.getSimpleVT(),
                                         CCValAssign::Full));
     }
   }
 
for (const auto &In : Ins) {
     assert(!In.Flags.isNest() && "nest is not valid for return values");
     if (In.Flags.isInAlloca())
       fail(DL, DAG, "WebAssembly hasn't implemented inalloca return values");
     if (In.Flags.isInConsecutiveRegs())
       fail(DL, DAG, "WebAssembly hasn't implemented cons regs return values");
     if (In.Flags.isInConsecutiveRegsLast())
       fail(DL, DAG,
            "WebAssembly hasn't implemented cons regs last return values");
-    // Ignore In.getOrigAlign() because all our arguments are passed in
+    // Ignore In.getNonZeroOrigAlign() because all our arguments are passed in
     // registers.
     InTys.push_back(In.VT);
   }
 
   if (CLI.IsTailCall) {
     // ret_calls do not return values to the current frame
     SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
     return DAG.getNode(WebAssemblyISD::RET_CALL, DL, NodeTys, Ops);
for (const ISD::InputArg &In : Ins) {
     if (In.Flags.isInAlloca())
       fail(DL, DAG, "WebAssembly hasn't implemented inalloca arguments");
     if (In.Flags.isNest())
       fail(DL, DAG, "WebAssembly hasn't implemented nest arguments");
     if (In.Flags.isInConsecutiveRegs())
       fail(DL, DAG, "WebAssembly hasn't implemented cons regs arguments");
     if (In.Flags.isInConsecutiveRegsLast())
       fail(DL, DAG, "WebAssembly hasn't implemented cons regs last arguments");
-    // Ignore In.getOrigAlign() because all our arguments are passed in
+    // Ignore In.getNonZeroOrigAlign() because all our arguments are passed in
     // registers.
     InVals.push_back(In.Used ? DAG.getNode(WebAssemblyISD::ARGUMENT, DL, In.VT,
                                            DAG.getTargetConstant(InVals.size(),
                                                                  DL, MVT::i32))
                              : DAG.getUNDEF(In.VT));
 
     // Record the number and types of arguments.
     MFI->addParam(In.VT);