[ms] [llvm-ml] Fix STRUCT field alignment

[epastor]

Summary

MASM aligns fields to the _minimum_ of the STRUCT alignment value and the size of the next field.

[epastor]

Sync to HEAD

[epastor]

Rebase to HEAD

llvm/lib/MC/MCParser/MasmParser.cpp

 struct StructInfo {
   StringRef Name;
   bool IsUnion = false;
   size_t Alignment = 0;
   size_t Size = 0;
   std::vector<FieldInfo> Fields;
   StringMap<size_t> FieldsByName;
 
-  FieldInfo &addField(StringRef FieldName, FieldType FT);
+  FieldInfo &addField(StringRef FieldName, FieldType FT, size_t FieldSize);
 
   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 of a single entry in this field, in bytes ("type" in MASM standards).
   size_t Type = 0;
 
   FieldInitializer Contents;
 
   FieldInfo(FieldType FT) : Contents(FT) {}
 };
 
-FieldInfo &StructInfo::addField(StringRef FieldName, FieldType FT) {
+FieldInfo &StructInfo::addField(StringRef FieldName, FieldType FT,
+                                size_t FieldSize) {
   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, Alignment);
+    Size = llvm::alignTo(Size, std::min(Alignment, FieldSize));
     Field.Offset = Size;
   }
   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.
private:
   bool emitIntegralValues(unsigned Size);
   bool addIntegralField(StringRef Name, unsigned Size);
   bool parseDirectiveValue(StringRef IDVal, unsigned Size);
   bool parseDirectiveNamedValue(StringRef IDVal, unsigned Size, StringRef Name,
                                 SMLoc NameLoc);
 
   // "real4", "real8"
   bool emitRealValues(const fltSemantics &Semantics);
-  bool addRealField(StringRef Name, const fltSemantics &Semantics);
-  bool parseDirectiveRealValue(StringRef IDVal, const fltSemantics &Semantics);
+  bool addRealField(StringRef Name, const fltSemantics &Semantics, size_t Size);
+  bool parseDirectiveRealValue(StringRef IDVal, const fltSemantics &Semantics,
+                               size_t Size);
   bool parseRealInstList(
       const fltSemantics &Semantics, SmallVectorImpl<APInt> &Values,
       const AsmToken::TokenKind EndToken = AsmToken::EndOfStatement);
   bool parseDirectiveNamedRealValue(StringRef IDVal,
-                                    const fltSemantics &Semantics,
+                                    const fltSemantics &Semantics, size_t Size,
                                     StringRef Name, SMLoc NameLoc);
 
   bool parseOptionalAngleBracketOpen();
   bool parseAngleBracketClose(const Twine &Msg = "expected '>'");
 
   bool parseFieldInitializer(const FieldInfo &Field,
                              FieldInitializer &Initializer);
   bool parseFieldInitializer(const FieldInfo &Field,
case DK_DD:
       return parseDirectiveValue(IDVal, 4);
     case DK_FWORD:
       return parseDirectiveValue(IDVal, 6);
     case DK_QWORD:
     case DK_SQWORD:
     case DK_DQ:
       return parseDirectiveValue(IDVal, 8);
     case DK_REAL4:
-      return parseDirectiveRealValue(IDVal, APFloat::IEEEsingle());
+      return parseDirectiveRealValue(IDVal, APFloat::IEEEsingle(), 4);
     case DK_REAL8:
-      return parseDirectiveRealValue(IDVal, APFloat::IEEEdouble());
+      return parseDirectiveRealValue(IDVal, APFloat::IEEEdouble(), 8);
     case DK_STRUCT:
     case DK_UNION:
       return parseDirectiveNestedStruct(IDVal, DirKind);
     case DK_ENDS:
       return parseDirectiveNestedEnds();
     case DK_ALIGN:
       return parseDirectiveAlign();
     case DK_ORG:
case DK_FWORD:
     Lex();
     return parseDirectiveNamedValue(nextVal, 6, IDVal, IDLoc);
   case DK_QWORD:
   case DK_DQ:
     Lex();
     return parseDirectiveNamedValue(nextVal, 8, IDVal, IDLoc);
   case DK_REAL4:
     Lex();
-    return parseDirectiveNamedRealValue(nextVal, APFloat::IEEEsingle(), IDVal,
-                                        IDLoc);
+    return parseDirectiveNamedRealValue(nextVal, APFloat::IEEEsingle(), 4,
+                                        IDVal, IDLoc);
   case DK_REAL8:
     Lex();
-    return parseDirectiveNamedRealValue(nextVal, APFloat::IEEEdouble(), IDVal,
-                                        IDLoc);
+    return parseDirectiveNamedRealValue(nextVal, APFloat::IEEEdouble(), 8,
+                                        IDVal, IDLoc);
   case DK_STRUCT:
   case DK_UNION:
     Lex();
     return parseDirectiveStruct(nextVal, DirKind, IDVal, IDLoc);
   case DK_ENDS:
     Lex();
     return parseDirectiveEnds(IDVal, IDLoc);
   }
for (auto Value : Values) {
     emitIntValue(Value, Size);
   }
   return false;
 }
 
 // Add a field to the current structure.
 bool MasmParser::addIntegralField(StringRef Name, unsigned Size) {
   StructInfo &Struct = StructInProgress.back();
-  FieldInfo &Field = Struct.addField(Name, FT_INTEGRAL);
+  FieldInfo &Field = Struct.addField(Name, FT_INTEGRAL, Size);
   IntFieldInfo &IntInfo = Field.Contents.IntInfo;
 
   Field.Type = Size;
 
   if (parseScalarInstList(Size, IntInfo.Values))
     return true;
 
   Field.SizeOf = Field.Type * IntInfo.Values.size();
bool MasmParser::emitRealValues(const fltSemantics &Semantics) {
   for (const APInt &AsInt : ValuesAsInt) {
     getStreamer().emitIntValue(AsInt.getLimitedValue(),
                                AsInt.getBitWidth() / 8);
   }
   return false;
 }
 
 // Add a real field to the current struct.
-bool MasmParser::addRealField(StringRef Name, const fltSemantics &Semantics) {
+bool MasmParser::addRealField(StringRef Name, const fltSemantics &Semantics,
+                              size_t Size) {
   StructInfo &Struct = StructInProgress.back();
-  FieldInfo &Field = Struct.addField(Name, FT_REAL);
+  FieldInfo &Field = Struct.addField(Name, FT_REAL, Size);
   RealFieldInfo &RealInfo = Field.Contents.RealInfo;
 
   Field.SizeOf = 0;
 
   if (parseRealInstList(Semantics, RealInfo.AsIntValues))
     return true;
 
   Field.Type = RealInfo.AsIntValues.back().getBitWidth() / 8;
   Field.LengthOf = RealInfo.AsIntValues.size();
   Field.SizeOf = Field.Type * Field.LengthOf;
   if (Struct.IsUnion)
     Struct.Size = std::max(Struct.Size, Field.SizeOf);
   else
     Struct.Size += Field.SizeOf;
   return false;
 }
 
 /// parseDirectiveRealValue
 ///  ::= (real4 | real8) [ expression (, expression)* ]
 bool MasmParser::parseDirectiveRealValue(StringRef IDVal,
-                                         const fltSemantics &Semantics) {
+                                         const fltSemantics &Semantics,
+                                         size_t Size) {
   if (StructInProgress.empty()) {
     // Initialize data value.
     if (emitRealValues(Semantics))
       return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
-  } else if (addRealField("", Semantics)) {
+  } else if (addRealField("", Semantics, Size)) {
     return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
   }
   return false;
 }
 
 /// parseDirectiveNamedRealValue
 ///  ::= name (real4 | real8) [ expression (, expression)* ]
 bool MasmParser::parseDirectiveNamedRealValue(StringRef IDVal,
                                               const fltSemantics &Semantics,
-                                              StringRef Name, SMLoc NameLoc) {
+                                              size_t Size, StringRef Name,
+                                              SMLoc NameLoc) {
   if (StructInProgress.empty()) {
     // Initialize named data value.
     MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
     getStreamer().emitLabel(Sym);
     if (emitRealValues(Semantics))
       return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
-  } else if (addRealField(Name, Semantics)) {
+  } else if (addRealField(Name, Semantics, Size)) {
     return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
   }
   return false;
 }
 
 bool MasmParser::parseOptionalAngleBracketOpen() {
   const AsmToken Tok = getTok();
   if (parseOptionalToken(AsmToken::LessLess)) {
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);
+  FieldInfo &Field = OwningStruct.addField(Name, FT_STRUCT, Structure.Size);
   StructFieldInfo &StructInfo = Field.Contents.StructInfo;
 
   StructInfo.Structure = Structure;
   Field.Type = Structure.Size;
 
   if (parseStructInstList(Structure, StructInfo.Initializers))
     return true;
 
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);
+    FieldInfo &Field =
+        ParentStruct.addField(Structure.Name, FT_STRUCT, Structure.Size);
     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.test

 ;
 ; DWORD 5
 ; CHECK-NEXT: .long 5
 ;
 ; BAZ <>
 ; CHECK-NEXT: .byte 1
 ; CHECK-NEXT: .byte 2
 ;
-; <BYTE 6, BYTE 7>, with internal alignment padding
+; <BYTE 6, BYTE 7>, with no alignment padding (field size < alignment)
 ; CHECK-NEXT: .byte 6
-; CHECK-NEXT: .zero 1
 ; CHECK-NEXT: .byte 7
-; CHECK-NEXT: .zero 1
 ;
 ; BYTE "abcde", plus alignment padding
 ; CHECK-NEXT: .byte 97
 ; CHECK-NEXT: .byte 98
 ; CHECK-NEXT: .byte 99
 ; CHECK-NEXT: .byte 100
 ; CHECK-NEXT: .byte 101
 ; CHECK-NEXT: .zero 1

 ;
 ; DWORD 5 (default-initialized when omitted)
 ; CHECK-NEXT: .long 5
 ;
 ; BAZ <10, 11>
 ; CHECK-NEXT: .byte 10
 ; CHECK-NEXT: .byte 11
 ;
-; <BYTE 6, BYTE 7>, with internal alignment padding
+; <BYTE 12, BYTE 7>, with no alignment padding (field size < alignment)
 ; CHECK-NEXT: .byte 12
-; CHECK-NEXT: .zero 1
 ; CHECK-NEXT: .byte 7
-; CHECK-NEXT: .zero 1
 ;
 ; BYTE "ijk", padded with " ", plus alignment padding
 ; CHECK-NEXT: .byte 105
 ; CHECK-NEXT: .byte 106
 ; CHECK-NEXT: .byte 107
 ; CHECK-NEXT: .byte 32
 ; CHECK-NEXT: .byte 32
 ; CHECK-NEXT: .zero 1
 
 .code
 
 t3:
 mov eax, t2.f.h
 mov eax, [t2].f.h
 mov eax, [t2.f.h]
 
 ; CHECK: t3:
-; CHECK-NEXT: mov eax, dword ptr [rip + t2+12]
-; CHECK-NEXT: mov eax, dword ptr [rip + t2+12]
-; CHECK-NEXT: mov eax, dword ptr [rip + t2+12]
+; CHECK-NEXT: mov eax, dword ptr [rip + t2+11]
+; CHECK-NEXT: mov eax, dword ptr [rip + t2+11]
+; CHECK-NEXT: mov eax, dword ptr [rip + t2+11]
 
 t4:
 mov eax, j.FOOBAR.f.h
 mov eax, j.baz.b
 
 ; CHECK: t4:
-; CHECK-NEXT: mov eax, dword ptr [rip + j+12]
+; CHECK-NEXT: mov eax, dword ptr [rip + j+11]
 ; CHECK-NEXT: mov eax, dword ptr [rip + j+1]
 
 t5:
 mov eax, [ebx].FOOBAR.f.h
 mov eax, [ebx.FOOBAR].f.h
 mov eax, [ebx.FOOBAR.f.h]
 
 ; CHECK: t5:
-; CHECK-NEXT: mov eax, dword ptr [ebx + 12]
-; CHECK-NEXT: mov eax, dword ptr [ebx + 12]
-; CHECK-NEXT: mov eax, dword ptr [ebx + 12]
+; CHECK-NEXT: mov eax, dword ptr [ebx + 11]
+; CHECK-NEXT: mov eax, dword ptr [ebx + 11]
+; CHECK-NEXT: mov eax, dword ptr [ebx + 11]
 
 t6:
 mov eax, t2.FOOBAR.f.h
 mov eax, [t2].FOOBAR.f.h
 mov eax, [t2.FOOBAR].f.h
 mov eax, [t2.FOOBAR.f.h]
 
 ; CHECK: t6:
-; CHECK-NEXT: mov eax, dword ptr [rip + t2+12]
-; CHECK-NEXT: mov eax, dword ptr [rip + t2+12]
-; CHECK-NEXT: mov eax, dword ptr [rip + t2+12]
-; CHECK-NEXT: mov eax, dword ptr [rip + t2+12]
+; CHECK-NEXT: mov eax, dword ptr [rip + t2+11]
+; CHECK-NEXT: mov eax, dword ptr [rip + t2+11]
+; CHECK-NEXT: mov eax, dword ptr [rip + t2+11]
+; CHECK-NEXT: mov eax, dword ptr [rip + t2+11]
 
 t7:
 mov eax, [ebx].FOOBAR.e.b
 mov eax, [ebx.FOOBAR].e.b
 mov eax, [ebx.FOOBAR.e].b
 mov eax, [ebx.FOOBAR.e.b]
 
 ; CHECK: t7:

 ; CHECK-NEXT: mov eax, dword ptr [ebx + 8]
 
 t10:
 mov eax, FOOBAR.f
 mov eax, FOOBAR.f.h
 
 ; CHECK: t10:
 ; CHECK-NEXT: mov eax, 10
-; CHECK-NEXT: mov eax, 12
+; CHECK-NEXT: mov eax, 11
 
 t11:
 mov eax, (FOOBAR PTR [ebx]).f
 mov eax, (FOOBAR PTR t1).f
 
 ; CHECK: t11:
 ; CHECK-NEXT: mov eax, dword ptr [ebx + 10]
 ; CHECK-NEXT: mov eax, dword ptr [rip + t1+10]
 
 END