[PATCH] D92507: [ms] [llvm-ml] Add support for ALIGN, EVEN, and ORG directives

[Nico Weber via Phabricator via llvm-commits]

thakis accepted this revision.
thakis added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:3539
   Field.LengthOf = IntInfo.Values.size();
-  if (Struct.IsUnion)
-    Struct.Size = std::max(Struct.Size, Field.SizeOf);
-  else
-    Struct.Size += Field.SizeOf;
+  Struct.Size = std::max(Struct.Size, Field.Offset + Field.SizeOf);
   return false;
----------------
we do this in a bunch of places -- maybe Struct should have a updateSizeForField()?

Many of these "add a field" functions look a bit repetitive -- not sure if that's the best way to cut back on that, or if there's a bigger win to be had.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:4392
   StructInfo &ParentStruct = StructInProgress.back();
   if (Structure.Name.empty()) {
     const size_t OldFields = ParentStruct.Fields.size();
----------------
can you add a comment here saying what an empty field name means? ("fields in nameless struct get added to containing struct")


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92507/new/

https://reviews.llvm.org/D92507