[ms] [llvm-ml] Fix struct padding logic

[epastor]

Summary

MASM structs are end-padded to have size a multiple of the smaller of the requested alignment and the size of their largest field (taken recursively, if they have a field of STRUCT type).

This matches the behavior of ml.exe and ml64.exe. Our original implementation followed the MASM 6.0 documentation, which instead specified that MASM structs were padded to a multiple of their requested alignment.

[epastor]

Rebase to HEAD

llvm/lib/MC/MCParser/MasmParser.cpp

 };
 
 struct FieldInfo;
 struct StructInfo {
   StringRef Name;
   bool IsUnion = false;
   size_t Alignment = 0;
   size_t Size = 0;
+  size_t AlignmentSize = 0;
   std::vector<FieldInfo> Fields;
   StringMap<size_t> FieldsByName;
 
-  FieldInfo &addField(StringRef FieldName, FieldType FT, size_t FieldSize);
+  FieldInfo &addField(StringRef FieldName, FieldType FT,
+                      size_t FieldAlignmentSize);
 
   StructInfo() = default;
 
   StructInfo(StringRef StructName, bool Union, unsigned AlignmentValue)
       : Name(StructName), IsUnion(Union), Alignment(AlignmentValue) {}
 };
 
 // FIXME: This should probably use a class hierarchy, raw pointers between the
struct FieldInfo {
   size_t Type = 0;
 
   FieldInitializer Contents;
 
   FieldInfo(FieldType FT) : Contents(FT) {}
 };
 
 FieldInfo &StructInfo::addField(StringRef FieldName, FieldType FT,
-                                size_t FieldSize) {
+                                size_t FieldAlignmentSize) {
   if (!FieldName.empty())
     FieldsByName[FieldName] = Fields.size();
   Fields.emplace_back(FT);
   FieldInfo &Field = Fields.back();
   if (IsUnion) {
     Field.Offset = 0;
   } else {
-    Size = llvm::alignTo(Size, std::min(Alignment, FieldSize));
+    Size = llvm::alignTo(Size, std::min(Alignment, FieldAlignmentSize));
     Field.Offset = Size;
   }
+  AlignmentSize = std::max(AlignmentSize, FieldAlignmentSize);
   return Field;
 }
 
 /// The concrete assembly parser instance.
 // Note that this is a full MCAsmParser, not an MCAsmParserExtension!
 // It's a peer of AsmParser, not of COFFAsmParser, WasmAsmParser, etc.
 class MasmParser : public MCAsmParser {
 private:
bool MasmParser::emitStructValues(const StructInfo &Structure) {
   }
 
   return false;
 }
 
 // Declare a field in the current struct.
 bool MasmParser::addStructField(StringRef Name, const StructInfo &Structure) {
   StructInfo &OwningStruct = StructInProgress.back();
-  FieldInfo &Field = OwningStruct.addField(Name, FT_STRUCT, Structure.Size);
+  FieldInfo &Field =
+      OwningStruct.addField(Name, FT_STRUCT, Structure.AlignmentSize);
   StructFieldInfo &StructInfo = Field.Contents.StructInfo;
 
   StructInfo.Structure = Structure;
   Field.Type = Structure.Size;
 
   if (parseStructInstList(Structure, StructInfo.Initializers))
     return true;
 
bool MasmParser::parseDirectiveEnds(StringRef Name, SMLoc NameLoc) {
   if (StructInProgress.empty())
     return Error(NameLoc, "ENDS directive without matching STRUC/STRUCT/UNION");
   if (StructInProgress.size() > 1)
     return Error(NameLoc, "unexpected name in nested ENDS directive");
   if (StructInProgress.back().Name.compare_lower(Name))
     return Error(NameLoc, "mismatched name in ENDS directive; expected '" +
                               StructInProgress.back().Name + "'");
   StructInfo Structure = StructInProgress.pop_back_val();
-  // Pad to make the structure's size divisible by its alignment.
-  Structure.Size = llvm::alignTo(Structure.Size, Structure.Alignment);
+  // Pad to make the structure's size divisible by the smaller of its alignment
+  // and the size of its largest field.
+  Structure.Size = llvm::alignTo(
+      Structure.Size, std::min(Structure.Alignment, Structure.AlignmentSize));
   Structs[Name.lower()] = Structure;
 
   if (parseToken(AsmToken::EndOfStatement))
     return addErrorSuffix(" in ENDS directive");
 
   return false;
 }
 
if (!ParentStruct.IsUnion) {
       }
     }
 
     if (ParentStruct.IsUnion)
       ParentStruct.Size = std::max(ParentStruct.Size, Structure.Size);
     else
       ParentStruct.Size += Structure.Size;
   } else {
-    FieldInfo &Field =
-        ParentStruct.addField(Structure.Name, FT_STRUCT, Structure.Size);
+    FieldInfo &Field = ParentStruct.addField(Structure.Name, FT_STRUCT,
+                                             Structure.AlignmentSize);
     StructFieldInfo &StructInfo = Field.Contents.StructInfo;
     Field.Type = Structure.Size;
     Field.LengthOf = 1;
     Field.SizeOf = Structure.Size;
 
     if (ParentStruct.IsUnion)
       ParentStruct.Size = std::max(ParentStruct.Size, Field.SizeOf);
     else

llvm/test/tools/llvm-ml/struct_alignment.test

+; RUN: llvm-ml -filetype=asm %s | FileCheck %s
+
+.data
+
+FOO STRUCT 8
+  f FWORD -1
+FOO ENDS
+
+t1 FOO <>
+; CHECK-LABEL: t1:
+; CHECK-NEXT: .long 4294967295
+; CHECK-NEXT: .short 65535
+; CHECK-NOT: .zero
+
+BAZ STRUCT
+  b BYTE 3 DUP (-1)
+  f FWORD -1
+BAZ ENDS
+
+FOOBAR STRUCT 8
+  f1 BAZ <>
+  f2 BAZ <>
+  h BYTE -1
+FOOBAR ENDS
+
+t2 FOOBAR <>
+; CHECK-LABEL: t2:
+; CHECK-NEXT: .byte -1
+; CHECK-NEXT: .byte -1
+; CHECK-NEXT: .byte -1
+; CHECK-NEXT: .long 4294967295
+; CHECK-NEXT: .short 65535
+; CHECK-NEXT: .zero 3
+; CHECK-NEXT: .byte -1
+; CHECK-NEXT: .byte -1
+; CHECK-NEXT: .byte -1
+; CHECK-NEXT: .long 4294967295
+; CHECK-NEXT: .short 65535
+; CHECK-NEXT: .byte -1
+; CHECK-NEXT: .zero 2
+
+.code
+
+END