apply suggestions

Author: el-ev

clang/lib/CIR/CodeGen/CIRGenExpr.cpp

@@ -319,24 +319,20 @@ LValue CIRGenFunction::emitLValueForField(LValue base, const FieldDecl *field) {
   unsigned recordCVR = base.getVRQualifiers();
 
   llvm::StringRef fieldName = field->getName();
+  unsigned fieldIndex;
+  assert(!cir::MissingFeatures::lambdaFieldToName());
 
-  if (rec->isUnion()) {
-    unsigned fieldIndex = field->getFieldIndex();
-    assert(!cir::MissingFeatures::lambdaFieldToName());
-    addr = emitAddrOfFieldStorage(addr, field, fieldName, fieldIndex);
-
-  } else {
-    assert(!cir::MissingFeatures::preservedAccessIndexRegion());
-
+  if (rec->isUnion())
+    fieldIndex = field->getFieldIndex();
+  else {
     const CIRGenRecordLayout &layout =
         cgm.getTypes().getCIRGenRecordLayout(field->getParent());
-    unsigned fieldIndex = layout.getCIRFieldNo(field);
-
-    assert(!cir::MissingFeatures::lambdaFieldToName());
-
-    addr = emitAddrOfFieldStorage(addr, field, fieldName, fieldIndex);
+    fieldIndex = layout.getCIRFieldNo(field);
   }
 
+  addr = emitAddrOfFieldStorage(addr, field, fieldName, fieldIndex);
+  assert(!cir::MissingFeatures::preservedAccessIndexRegion());
+
   // If this is a reference field, load the reference right now.
   if (fieldType->isReferenceType()) {
     cgm.errorNYI(field->getSourceRange(), "emitLValueForField: reference type");

clang/lib/CIR/CodeGen/CIRGenRecordLayout.h

@@ -31,7 +31,7 @@ class CIRGenRecordLayout {
 
   /// Map from (non-bit-field) record field to the corresponding cir record type
   /// field no. This info is populated by the record builder.
-  llvm::DenseMap<const clang::FieldDecl *, unsigned> fieldInfo;
+  llvm::DenseMap<const clang::FieldDecl *, unsigned> fieldIdxMap;
 
 public:
   CIRGenRecordLayout(cir::RecordType completeObjectType)
@@ -44,8 +44,8 @@ class CIRGenRecordLayout {
   /// Return cir::RecordType element number that corresponds to the field FD.
   unsigned getCIRFieldNo(const clang::FieldDecl *fd) const {
     fd = fd->getCanonicalDecl();
-    assert(fieldInfo.count(fd) && "Invalid field for record!");
-    return fieldInfo.lookup(fd);
+    assert(fieldIdxMap.count(fd) && "Invalid field for record!");
+    return fieldIdxMap.lookup(fd);
   }
 };
 

clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp

@@ -142,7 +142,7 @@ struct CIRRecordLowering final {
   std::vector<MemberInfo> members;
   // Output fields, consumed by CIRGenTypes::computeRecordLayout
   llvm::SmallVector<mlir::Type, 16> fieldTypes;
-  llvm::DenseMap<const FieldDecl *, unsigned> fields;
+  llvm::DenseMap<const FieldDecl *, unsigned> fieldIdxMap;
   cir::CIRDataLayout dataLayout;
 
   LLVM_PREFERRED_TYPE(bool)
@@ -208,7 +208,8 @@ void CIRRecordLowering::fillOutputFields() {
       fieldTypes.push_back(member.data);
     if (member.kind == MemberInfo::InfoKind::Field) {
       if (member.fieldDecl)
-        fields[member.fieldDecl->getCanonicalDecl()] = fieldTypes.size() - 1;
+        fieldIdxMap[member.fieldDecl->getCanonicalDecl()] =
+            fieldTypes.size() - 1;
       // A field without storage must be a bitfield.
       assert(!cir::MissingFeatures::bitfields());
     }
@@ -310,7 +311,7 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *rd, cir::RecordType *ty) {
   assert(!cir::MissingFeatures::bitfields());
 
   // Add all the field numbers.
-  rl->fieldInfo.swap(lowering.fields);
+  rl->fieldIdxMap.swap(lowering.fieldIdxMap);
 
   // Dump the layout, if requested.
   if (getASTContext().getLangOpts().DumpRecordLayouts) {
@@ -327,10 +328,7 @@ void CIRRecordLowering::lowerUnion() {
   bool seenNamedMember = false;
 
   // Iterate through the fields setting bitFieldInfo and the Fields array. Also
-  // locate the "most appropriate" storage type.  The heuristic for finding the
-  // storage type isn't necessary, the first (non-0-length-bitfield) field's
-  // type would work fine and be simpler but would be different than what we've
-  // been doing and cause lit tests to change.
+  // locate the "most appropriate" storage type.
   for (const FieldDecl *field : recordDecl->fields()) {
     mlir::Type fieldType;
     if (field->isBitField())
@@ -339,12 +337,13 @@ void CIRRecordLowering::lowerUnion() {
     else
       fieldType = getStorageType(field);
 
-    fields[field->getCanonicalDecl()] = 0;
+    // This maps a field to its index. For unions, the index is always 0.
+    fieldIdxMap[field->getCanonicalDecl()] = 0;
 
     // Compute zero-initializable status.
     // This union might not be zero initialized: it may contain a pointer to
     // data member which might have some exotic initialization sequence.
-    // If this is the case, then we aught not to try and come up with a "better"
+    // If this is the case, then we ought not to try and come up with a "better"
     // type, it might not be very easy to come up with a Constant which
     // correctly initializes it.
     if (!seenNamedMember) {
@@ -380,9 +379,8 @@ void CIRRecordLowering::lowerUnion() {
 
   if (layoutSize < getSize(storageType))
     storageType = getByteArrayType(layoutSize);
-
-  // NOTE(cir): Defer padding calculations to the lowering process.
-  appendPaddingBytes(layoutSize - getSize(storageType));
+  else
+    appendPaddingBytes(layoutSize - getSize(storageType));
 
   // Set packed if we need it.
   if (layoutSize % getAlignment(storageType))

clang/lib/CIR/Dialect/IR/CIRTypes.cpp

@@ -235,25 +235,18 @@ void RecordType::complete(ArrayRef<Type> members, bool packed, bool padded) {
 /// Recurses into union members never returning a union as the largest member.
 Type RecordType::getLargestMember(const ::mlir::DataLayout &dataLayout) const {
   assert(isUnion() && "Only call getLargestMember on unions");
-  Type largestMember;
-  unsigned largestMemberSize = 0;
-  unsigned numElements = getNumElements();
-  auto members = getMembers();
-  if (getPadded())
-    numElements -= 1; // The last element is padding.
-  for (unsigned i = 0; i < numElements; ++i) {
-    Type ty = members[i];
-    if (!largestMember ||
-        dataLayout.getTypeABIAlignment(ty) >
-            dataLayout.getTypeABIAlignment(largestMember) ||
-        (dataLayout.getTypeABIAlignment(ty) ==
-             dataLayout.getTypeABIAlignment(largestMember) &&
-         dataLayout.getTypeSize(ty) > largestMemberSize)) {
-      largestMember = ty;
-      largestMemberSize = dataLayout.getTypeSize(largestMember);
-    }
-  }
-  return largestMember;
+  llvm::ArrayRef<Type> members = getMembers();
+  // If the union is padded, we need to ignore the last member,
+  // which is the padding.
+  return *std::max_element(
+      members.begin(), getPadded() ? members.end() - 1 : members.end(),
+      [&](Type lhs, Type rhs) {
+        return dataLayout.getTypeABIAlignment(lhs) <
+                   dataLayout.getTypeABIAlignment(rhs) ||
+               (dataLayout.getTypeABIAlignment(lhs) ==
+                    dataLayout.getTypeABIAlignment(rhs) &&
+                dataLayout.getTypeSize(lhs) < dataLayout.getTypeSize(rhs));
+      });
 }
 
 //===----------------------------------------------------------------------===//