[Alignment][NFC] Use Align for BPFAbstractMemberAccess::RecordAlignment

[gchatelet]

Summary

This 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]

llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp

120

I cannot convince myself that this is an NFC. Aggregate initialization will initialize RecordAlignment to 0, and there are some cases when RecordAlignment is not set by code (e.g. BPFAbstractMemberAccess::IsPreserveDIAccessIndexCall).

On the other hand, the only usage is

uint32_t MemberBitSize = MemberTy->getSizeInBits();
  uint32_t MemberBitOffset = MemberTy->getOffsetInBits();
  uint32_t AlignBits = RecordAlignment * 8;
  if (RecordAlignment > 8 || MemberBitSize > AlignBits)
    report_fatal_error("Unsupported field expression for llvm.bpf.preserve.field.info, "
                       "requiring too big alignment");

So it looks like 0 would always lead to a fatal error anyway, so non-zero is really not intended.

[gchatelet]

llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp

120

I find this puzzling because the error specifically mentions "llvm.bpf.preserve.field.info" and if you look at IsPreserveDIAccessIndexCall, the only case where RecordAlignment can be 0 is when GV->getName().startswith("llvm.bpf.preserve.field.info"). On all other cases RecordAlignment>0 and GV->getName() would start with one of:

• "llvm.preserve.array.access.index"
• "llvm.preserve.union.access.index"
• "llvm.preserve.struct.access.index"

So it doesn't make sense to me.

As you said, 0 would lead to a fatal error so I take this as RecordAlignment!=0.

llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp

public:
   // Add optional BPFTargetMachine parameter so that BPF backend can add the phase
   // with target machine to find out the endianness. The default constructor (without
   // parameters) is used by the pass manager for managing purposes.
   BPFAbstractMemberAccess(BPFTargetMachine *TM = nullptr) : ModulePass(ID), TM(TM) {}
 
   struct CallInfo {
     uint32_t Kind;
     uint32_t AccessIndex;
-    uint32_t RecordAlignment;
+    Align RecordAlignment;
     MDNode *Metadata;
     Value *Base;
   };
   typedef std::stack<std::pair<CallInst *, CallInfo>> CallInfoStack;
 
 private:
   enum : uint32_t {
     BPFPreserveArrayAI = 1,
private:
 
   bool IsPreserveDIAccessIndexCall(const CallInst *Call, CallInfo &Cinfo);
   bool IsValidAIChain(const MDNode *ParentMeta, uint32_t ParentAI,
                       const MDNode *ChildMeta);
   bool removePreserveAccessIndexIntrinsic(Module &M);
   void replaceWithGEP(std::vector<CallInst *> &CallList,
                       uint32_t NumOfZerosIndex, uint32_t DIIndex);
   bool HasPreserveFieldInfoCall(CallInfoStack &CallStack);
-  void GetStorageBitRange(DIDerivedType *MemberTy, uint32_t RecordAlignment,
+  void GetStorageBitRange(DIDerivedType *MemberTy, Align RecordAlignment,
                           uint32_t &StartBitOffset, uint32_t &EndBitOffset);
   uint32_t GetFieldInfo(uint32_t InfoKind, DICompositeType *CTy,
                         uint32_t AccessIndex, uint32_t PatchImm,
-                        uint32_t RecordAlignment);
+                        Align RecordAlignment);
 
   Value *computeBaseAndAccessKey(CallInst *Call, CallInfo &CInfo,
                                  std::string &AccessKey, MDNode *&BaseMeta);
   uint64_t getConstant(const Value *IndexValue);
   bool transformGEPChain(Module &M, CallInst *Call, CallInfo &CInfo);
 };
 } // End anonymous namespace
 
bool BPFAbstractMemberAccess::IsPreserveDIAccessIndexCall(const CallInst *Call,
   if (GV->getName().startswith("llvm.preserve.array.access.index")) {
     CInfo.Kind = BPFPreserveArrayAI;
     CInfo.Metadata = Call->getMetadata(LLVMContext::MD_preserve_access_index);
     if (!CInfo.Metadata)
       report_fatal_error("Missing metadata for llvm.preserve.array.access.index intrinsic");
     CInfo.AccessIndex = getConstant(Call->getArgOperand(2));
     CInfo.Base = Call->getArgOperand(0);
     CInfo.RecordAlignment =
-        DL->getABITypeAlignment(CInfo.Base->getType()->getPointerElementType());
+        DL->getABITypeAlign(CInfo.Base->getType()->getPointerElementType());
     return true;
   }
   if (GV->getName().startswith("llvm.preserve.union.access.index")) {
     CInfo.Kind = BPFPreserveUnionAI;
     CInfo.Metadata = Call->getMetadata(LLVMContext::MD_preserve_access_index);
     if (!CInfo.Metadata)
       report_fatal_error("Missing metadata for llvm.preserve.union.access.index intrinsic");
     CInfo.AccessIndex = getConstant(Call->getArgOperand(1));
     CInfo.Base = Call->getArgOperand(0);
     CInfo.RecordAlignment =
-        DL->getABITypeAlignment(CInfo.Base->getType()->getPointerElementType());
+        DL->getABITypeAlign(CInfo.Base->getType()->getPointerElementType());
     return true;
   }
   if (GV->getName().startswith("llvm.preserve.struct.access.index")) {
     CInfo.Kind = BPFPreserveStructAI;
     CInfo.Metadata = Call->getMetadata(LLVMContext::MD_preserve_access_index);
     if (!CInfo.Metadata)
       report_fatal_error("Missing metadata for llvm.preserve.struct.access.index intrinsic");
     CInfo.AccessIndex = getConstant(Call->getArgOperand(2));
     CInfo.Base = Call->getArgOperand(0);
     CInfo.RecordAlignment =
-        DL->getABITypeAlignment(CInfo.Base->getType()->getPointerElementType());
+        DL->getABITypeAlign(CInfo.Base->getType()->getPointerElementType());
     return true;
   }
   if (GV->getName().startswith("llvm.bpf.preserve.field.info")) {
     CInfo.Kind = BPFPreserveFieldInfoAI;
     CInfo.Metadata = nullptr;
     // Check validity of info_kind as clang did not check this.
     uint64_t InfoKind = getConstant(Call->getArgOperand(1));
     if (InfoKind >= BPFCoreSharedInfo::MAX_FIELD_RELOC_KIND)

 uint64_t BPFAbstractMemberAccess::getConstant(const Value *IndexValue) {
   const ConstantInt *CV = dyn_cast<ConstantInt>(IndexValue);
   assert(CV);
   return CV->getValue().getZExtValue();
 }
 
 /// Get the start and the end of storage offset for \p MemberTy.
 void BPFAbstractMemberAccess::GetStorageBitRange(DIDerivedType *MemberTy,
-                                                 uint32_t RecordAlignment,
+                                                 Align RecordAlignment,
                                                  uint32_t &StartBitOffset,
                                                  uint32_t &EndBitOffset) {
   uint32_t MemberBitSize = MemberTy->getSizeInBits();
   uint32_t MemberBitOffset = MemberTy->getOffsetInBits();
-  uint32_t AlignBits = RecordAlignment * 8;
+  uint32_t AlignBits = RecordAlignment.value() * 8;
   if (RecordAlignment > 8 || MemberBitSize > AlignBits)
     report_fatal_error("Unsupported field expression for llvm.bpf.preserve.field.info, "
                        "requiring too big alignment");
 
   StartBitOffset = MemberBitOffset & ~(AlignBits - 1);
   if ((StartBitOffset + AlignBits) < (MemberBitOffset + MemberBitSize))
     report_fatal_error("Unsupported field expression for llvm.bpf.preserve.field.info, "
                        "cross alignment boundary");
   EndBitOffset = StartBitOffset + AlignBits;
 }
 
 uint32_t BPFAbstractMemberAccess::GetFieldInfo(uint32_t InfoKind,
                                                DICompositeType *CTy,
                                                uint32_t AccessIndex,
                                                uint32_t PatchImm,
-                                               uint32_t RecordAlignment) {
+                                               Align RecordAlignment) {
   if (InfoKind == BPFCoreSharedInfo::FIELD_EXISTENCE)
       return 1;
 
   uint32_t Tag = CTy->getTag();
   if (InfoKind == BPFCoreSharedInfo::FIELD_BYTE_OFFSET) {
     if (Tag == dwarf::DW_TAG_array_type) {
       auto *EltTy = stripQualifiers(CTy->getBaseType());
       PatchImm += AccessIndex * calcArraySize(CTy, 1) *
if (CallStack.size()) {
       }
     }
 
     // Access Index
     uint64_t AccessIndex = CInfo.AccessIndex;
     AccessKey += ":" + std::to_string(AccessIndex);
 
     MDNode *MDN = CInfo.Metadata;
-    uint32_t RecordAlignment = CInfo.RecordAlignment;
     // At this stage, it cannot be pointer type.
     auto *CTy = cast<DICompositeType>(stripQualifiers(cast<DIType>(MDN)));
     PatchImm = GetFieldInfo(InfoKind, CTy, AccessIndex, PatchImm,
-                            RecordAlignment);
+                            CInfo.RecordAlignment);
   }
 
   // Access key is the
   //   "llvm." + type name + ":" + reloc type + ":" + patched imm + "$" +
   //   access string,
   // uniquely identifying one relocation.
   // The prefix "llvm." indicates this is a temporary global, which should
   // not be emitted to ELF file.