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

[epastor]

Summary

Match ML.EXE's behavior for ALIGN, EVEN, and ORG directives both at file level and in STRUCTs.

We currently reject negative offsets passed to ORG inside STRUCTs (in ML.EXE and ML64.EXE, they wrap around as for an unsigned 32-bit integer).

Also, if a STRUCT is declared using an ORG directive, no value of that type can be defined.

[thakis]

Does it wrap around 32-bit in 64-bit output too?

Is this relied on? Or could we error out on negative values instead?

[thakis]

(marking "request changes" just so it disappears form my dashboard and can reappear once you've answered -- doesn't necessarily need changes)

[epastor]

In D92507#2431130, @thakis wrote:

Does it wrap around 32-bit in 64-bit output too?

Oddly enough: yes.

Is this relied on? Or could we error out on negative values instead?

I don't think we can prove that it isn't relied on, especially when ORG explicitly allows putting things "outside bounds", so I'd rather maintain compatibility.

[epastor]

Rebase on parent

[thakis]

In D92507#2445774, @epastor wrote:

In D92507#2431130, @thakis wrote:

Does it wrap around 32-bit in 64-bit output too?

Oddly enough: yes.

Is this relied on? Or could we error out on negative values instead?

I don't think we can prove that it isn't relied on, especially when ORG explicitly allows putting things "outside bounds", so I'd rather maintain compatibility.

Well, we don't need to prove it if we make it an error: We'll know if the diag fires. So if we make this an error first and then build lots of files with it, we'll learn if we run into it. At that point we'll know we need to make it compatible, but maybe we won't have to. Doesn't it seem better to default to less surprising behavior and only put in the compat warts once there's evidence that we need them?

[thakis]

(marking "request changes" just so it disappears form my dashboard and can reappear once you've answered -- doesn't necessarily need changes)

[epastor]

Emit an error on negative ORG offsets, per feedback.

We can switch this to a warning if we find that real MASM code relies on the 32-bit wraparound found in ml.exe.

[epastor]

Restore the explicit cast to make it easier to switch to a Warning later

[thakis]

llvm/lib/MC/MCParser/MasmParser.cpp

3539

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.

4392

can you add a comment here saying what an empty field name means? ("fields in nameless struct get added to containing struct")

[epastor]

In D92507#2448819, @thakis wrote:

In D92507#2445774, @epastor wrote:

In D92507#2431130, @thakis wrote:

Does it wrap around 32-bit in 64-bit output too?

Oddly enough: yes.

Is this relied on? Or could we error out on negative values instead?

I don't think we can prove that it isn't relied on, especially when ORG explicitly allows putting things "outside bounds", so I'd rather maintain compatibility.

Well, we don't need to prove it if we make it an error: We'll know if the diag fires. So if we make this an error first and then build lots of files with it, we'll learn if we run into it. At that point we'll know we need to make it compatible, but maybe we won't have to. Doesn't it seem better to default to less surprising behavior and only put in the compat warts once there's evidence that we need them?

I'm not convinced on the philosophical grounds... but I went poking around GitHub for evidence, and couldn't find any uses of ORG inside a STRUCT with a negative offset. So - made into an error!

[epastor]

While I was working on the comments above, I discovered a problem - after improving the tests and fixing various errors, the way we were dealing with determining field offsets caused a failure on the second field after an ORG offset change. I've been working on and off to adjust the code to handle this; it requires a nontrivial rework, as Structure has to track both its Size and the offset of the next field. I'll upload that revision shortly, and I'm afraid it will require a re-review. Sorry, thakis - and I'll try to find someone else if more appropriate.

[epastor]

Fix handling of the second (and later) field after an ORG directive

This required a nontrivial rework of how we handle offset tracking - but I think it simplifies the code significantly.

[epastor]

llvm/lib/MC/MCParser/MasmParser.cpp

3539

We should definitely do a rework of this. I'll see what I can do in a followup commit.

[thakis]

llvm/lib/MC/MCParser/MasmParser.cpp

4485

This is never set anywhere in this function. Remove, and just return false at the end? And I guess the comment in the line above is wrong?

Or if you meant to ReturnVal = true instead of return true on line 4576, add a test that shows why that's important.

Also, please use a better name than ReturnVal then.

4552

Do you expect parseDirectiveEven() to become longer in the future? Why doesn't this do

if (emitAlignTo(2))
  return addErrorSuffix(" in even directive");

[epastor]

Addressed comments (removed copy/paste cruft that proved unnecessary)

[epastor]

Added comment explaining anonymous substructures

llvm/lib/MC/MCParser/MasmParser.cpp

4392

Added for clarity.

4552

Simplified. Thanks!

[epastor]

Rebase on trunk to re-trigger build (after removing parent commit)

[thakis]

lgtm.

llvm/lib/MC/MCParser/MasmParser.cpp

enum FieldType {
   FT_REAL,     // Initializer: real number, stored as an APInt.
   FT_STRUCT    // Initializer: struct initializer, stored recursively.
 };
 
 struct FieldInfo;
 struct StructInfo {
   StringRef Name;
   bool IsUnion = false;
+  bool Initializable = true;
   unsigned Alignment = 0;
-  unsigned Size = 0;
   unsigned AlignmentSize = 0;
+  Optional<unsigned> NextFieldOffset;
+  unsigned Size = 0;
   std::vector<FieldInfo> Fields;
   StringMap<size_t> FieldsByName;
 
   FieldInfo &addField(StringRef FieldName, FieldType FT,
                       unsigned FieldAlignmentSize);
 
   StructInfo() = default;
 

 };
 
 struct StructInitializer {
   std::vector<FieldInitializer> FieldInitializers;
 };
 
 struct FieldInfo {
   // Offset of the field within the containing STRUCT.
-  size_t Offset = 0;
+  unsigned Offset = 0;
 
   // Total size of the field (= LengthOf * Type).
   unsigned SizeOf = 0;
 
   // Number of elements in the field (1 if scalar, >1 if an array).
   unsigned LengthOf = 0;
 
   // Size of a single entry in this field, in bytes ("type" in MASM standards).
   unsigned Type = 0;
 
   FieldInitializer Contents;
 
   FieldInfo(FieldType FT) : Contents(FT) {}
 };
 
 FieldInfo &StructInfo::addField(StringRef FieldName, FieldType FT,
                                 unsigned FieldAlignmentSize) {
   if (!FieldName.empty())
     FieldsByName[FieldName.lower()] = Fields.size();
   Fields.emplace_back(FT);
   FieldInfo &Field = Fields.back();
   if (IsUnion) {
     Field.Offset = 0;
+  } else if (NextFieldOffset.hasValue()) {
+    Field.Offset = NextFieldOffset.getValue();
+    NextFieldOffset.reset();
+    Size = std::max(Size, Field.Offset);
   } else {
-    Size = llvm::alignTo(Size, std::min(Alignment, FieldAlignmentSize));
-    Field.Offset = Size;
+    Field.Offset = llvm::alignTo(Size, std::min(Alignment, FieldAlignmentSize));
+    Size = std::max(Size, Field.Offset);
   }
   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.
enum DirectiveKind {
     DK_DD,
     DK_DF,
     DK_DQ,
     DK_DW,
     DK_REAL4,
     DK_REAL8,
     DK_REAL10,
     DK_ALIGN,
+    DK_EVEN,
     DK_ORG,
     DK_ENDR,
     DK_EXTERN,
     DK_PUBLIC,
     DK_COMM,
     DK_COMMENT,
     DK_INCLUDE,
     DK_REPEAT,
private:
   bool parseDirectiveNamedStructValue(const StructInfo &Structure,
                                       StringRef Directive, SMLoc DirLoc,
                                       StringRef Name);
 
   // "=", "equ", "textequ"
   bool parseDirectiveEquate(StringRef IDVal, StringRef Name,
                             DirectiveKind DirKind);
 
-  bool parseDirectiveOrg(); // ".org"
+  bool parseDirectiveOrg(); // "org"
+
+  bool emitAlignTo(int64_t Alignment);
   bool parseDirectiveAlign();  // "align"
+  bool parseDirectiveEven();   // "even"
 
   // ".file", ".line", ".loc", ".stabs"
   bool parseDirectiveFile(SMLoc DirectiveLoc);
   bool parseDirectiveLine();
   bool parseDirectiveLoc();
   bool parseDirectiveStabs();
 
   // ".cv_file", ".cv_func_id", ".cv_inline_site_id", ".cv_loc", ".cv_linetable",
case DK_REAL10:
       return parseDirectiveRealValue(IDVal, APFloat::x87DoubleExtended(), 10);
     case DK_STRUCT:
     case DK_UNION:
       return parseDirectiveNestedStruct(IDVal, DirKind);
     case DK_ENDS:
       return parseDirectiveNestedEnds();
     case DK_ALIGN:
       return parseDirectiveAlign();
+    case DK_EVEN:
+      return parseDirectiveEven();
     case DK_ORG:
       return parseDirectiveOrg();
     case DK_EXTERN:
       eatToEndOfStatement(); // .extern is the default, ignore it.
       return false;
     case DK_PUBLIC:
       return parseDirectiveSymbolAttribute(MCSA_Global);
     case DK_COMM:
bool MasmParser::addIntegralField(StringRef Name, unsigned Size) {
 
   Field.Type = Size;
 
   if (parseScalarInstList(Size, IntInfo.Values))
     return true;
 
   Field.SizeOf = Field.Type * IntInfo.Values.size();
   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;
 }
 
 /// parseDirectiveValue
 ///  ::= (byte | word | ... ) [ expression (, expression)* ]
 bool MasmParser::parseDirectiveValue(StringRef IDVal, unsigned Size) {
   if (StructInProgress.empty()) {
     // Initialize data value.
bool MasmParser::addRealField(StringRef Name, const fltSemantics &Semantics,
   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;
+  Struct.Size = std::max(Struct.Size, Field.Offset + Field.SizeOf);
   return false;
 }
 
 /// parseDirectiveRealValue
 ///  ::= (real4 | real8 | real10) [ expression (, expression)* ]
 bool MasmParser::parseDirectiveRealValue(StringRef IDVal,
                                          const fltSemantics &Semantics,
                                          size_t Size) {
bool MasmParser::emitFieldInitializer(const FieldInfo &Field,
   }
   return false;
 }
 
 bool MasmParser::emitFieldInitializer(const FieldInfo &Field,
                                       const StructFieldInfo &Contents,
                                       const StructFieldInfo &Initializer) {
   for (const auto &Init : Initializer.Initializers) {
-    emitStructInitializer(Contents.Structure, Init);
+    if (emitStructInitializer(Contents.Structure, Init))
+      return true;
   }
   // Default-initialize all remaining values.
   for (auto It =
            Contents.Initializers.begin() + Initializer.Initializers.size();
        It != Contents.Initializers.end(); ++It) {
     const auto &Init = *It;
-    emitStructInitializer(Contents.Structure, Init);
+    if (emitStructInitializer(Contents.Structure, Init))
+      return true;
   }
   return false;
 }
 
 bool MasmParser::emitFieldInitializer(const FieldInfo &Field,
                                       const FieldInitializer &Initializer) {
   switch (Field.Contents.FT) {
   case FT_INTEGRAL:
     return emitFieldInitializer(Field, Field.Contents.IntInfo,
                                 Initializer.IntInfo);
   case FT_REAL:
     return emitFieldInitializer(Field, Field.Contents.RealInfo,
                                 Initializer.RealInfo);
   case FT_STRUCT:
     return emitFieldInitializer(Field, Field.Contents.StructInfo,
                                 Initializer.StructInfo);
   }
   llvm_unreachable("Unhandled FieldType enum");
 }
 
 bool MasmParser::emitStructInitializer(const StructInfo &Structure,
                                        const StructInitializer &Initializer) {
+  if (!Structure.Initializable)
+    return Error(getLexer().getLoc(),
+                 "cannot initialize a value of type '" + Structure.Name +
+                     "'; 'org' was used in the type's declaration");
   size_t Index = 0, Offset = 0;
   for (const auto &Init : Initializer.FieldInitializers) {
     const auto &Field = Structure.Fields[Index++];
     getStreamer().emitZeros(Field.Offset - Offset);
     Offset = Field.Offset + Field.SizeOf;
     if (emitFieldInitializer(Field, Init))
       return true;
   }
bool MasmParser::addStructField(StringRef Name, const StructInfo &Structure) {
   StructInfo.Structure = Structure;
   Field.Type = Structure.Size;
 
   if (parseStructInstList(Structure, StructInfo.Initializers))
     return true;
 
   Field.LengthOf = StructInfo.Initializers.size();
   Field.SizeOf = Field.Type * Field.LengthOf;
-  if (OwningStruct.IsUnion)
-    OwningStruct.Size = std::max(OwningStruct.Size, Field.SizeOf);
-  else
-    OwningStruct.Size += Field.SizeOf;
+  OwningStruct.Size = std::max(OwningStruct.Size, Field.Offset + Field.SizeOf);
 
   return false;
 }
 
 /// parseDirectiveStructValue
 ///  ::= struct-id (<struct-initializer> | {struct-initializer})
 ///                [, (<struct-initializer> | {struct-initializer})]*
 bool MasmParser::parseDirectiveStructValue(const StructInfo &Structure,
bool MasmParser::parseDirectiveNestedEnds() {
   if (parseToken(AsmToken::EndOfStatement))
     return addErrorSuffix(" in nested ENDS directive");
 
   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);
 
   StructInfo &ParentStruct = StructInProgress.back();
   if (Structure.Name.empty()) {
     const size_t OldFields = ParentStruct.Fields.size();
     ParentStruct.Fields.insert(
         ParentStruct.Fields.end(),
         std::make_move_iterator(Structure.Fields.begin()),
         std::make_move_iterator(Structure.Fields.end()));
     for (const auto &FieldByName : Structure.FieldsByName) {
       ParentStruct.FieldsByName[FieldByName.getKey()] =
           FieldByName.getValue() + OldFields;
     }
-    if (!ParentStruct.IsUnion) {
-      for (auto FieldIter = ParentStruct.Fields.begin() + OldFields;
-           FieldIter != ParentStruct.Fields.end(); ++FieldIter) {
-        FieldIter->Offset += ParentStruct.Size;
+
+    unsigned FirstFieldOffset = 0;
+    if (!Structure.Fields.empty() && !ParentStruct.IsUnion) {
+      if (ParentStruct.NextFieldOffset.hasValue()) {
+        FirstFieldOffset = ParentStruct.NextFieldOffset.getValue();
+      } else {
+        FirstFieldOffset =
+            llvm::alignTo(ParentStruct.Size, std::min(ParentStruct.Alignment,
+                                                      Structure.AlignmentSize));
       }
     }
 
-    if (ParentStruct.IsUnion)
+    if (ParentStruct.IsUnion) {
       ParentStruct.Size = std::max(ParentStruct.Size, Structure.Size);
-    else
-      ParentStruct.Size += Structure.Size;
+    } else {
+      for (auto FieldIter = ParentStruct.Fields.begin() + OldFields;
+           FieldIter != ParentStruct.Fields.end(); ++FieldIter) {
+        FieldIter->Offset += FirstFieldOffset;
+      }
+      ParentStruct.Size =
+          std::max(ParentStruct.Size, FirstFieldOffset + Structure.Size);
+    }
   } else {
     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
-      ParentStruct.Size += Field.SizeOf;
+    ParentStruct.Size =
+        std::max(ParentStruct.Size, Field.Offset + Field.SizeOf);
 
     StructInfo.Structure = Structure;
     StructInfo.Initializers.emplace_back();
     auto &FieldInitializers = StructInfo.Initializers.back().FieldInitializers;
     for (const auto &SubField : Structure.Fields) {
       FieldInitializers.push_back(SubField.Contents);
     }
   }
 
   return false;
 }
 
 /// parseDirectiveOrg
-///  ::= .org expression [ , expression ]
+///  ::= org expression
 bool MasmParser::parseDirectiveOrg() {
   const MCExpr *Offset;
   SMLoc OffsetLoc = Lexer.getLoc();
   if (checkForValidSection() || parseExpression(Offset))
     return true;
-
-  // Parse optional fill expression.
-  int64_t FillExpr = 0;
-  if (parseOptionalToken(AsmToken::Comma))
-    if (parseAbsoluteExpression(FillExpr))
-      return addErrorSuffix(" in '.org' directive");
   if (parseToken(AsmToken::EndOfStatement))
-    return addErrorSuffix(" in '.org' directive");
+    return addErrorSuffix(" in 'org' directive");
+
+  if (StructInProgress.empty()) {
+    // Not in a struct; change the offset for the next instruction or data
+    if (checkForValidSection())
+      return addErrorSuffix(" in 'org' directive");
+
+    getStreamer().emitValueToOffset(Offset, 0, OffsetLoc);
+  } else {
+    // Offset the next field of this struct
+    StructInfo &Structure = StructInProgress.back();
+    int64_t OffsetRes;
+    if (!Offset->evaluateAsAbsolute(OffsetRes, getStreamer().getAssemblerPtr()))
+      return Error(OffsetLoc,
+                   "expected absolute expression in 'org' directive");
+    if (OffsetRes < 0)
+      return Error(
+          OffsetLoc,
+          "expected non-negative value in struct's 'org' directive; was " +
+              std::to_string(OffsetRes));
+    Structure.NextFieldOffset = static_cast<unsigned>(OffsetRes);
+
+    // ORG-affected structures cannot be initialized
+    Structure.Initializable = false;
+  }
 
-  getStreamer().emitValueToOffset(Offset, FillExpr, OffsetLoc);
   return false;
 }
 
+bool MasmParser::emitAlignTo(int64_t Alignment) {
+  // Always emit an alignment here even if we throw an error.
+  bool ReturnVal = false;
+
+  if (StructInProgress.empty()) {
+    // Not in a struct; align the next instruction or data
+    if (checkForValidSection())
+      return true;
+
+    // Check whether we should use optimal code alignment for this align
+    // directive.
+    const MCSection *Section = getStreamer().getCurrentSectionOnly();
+    assert(Section && "must have section to emit alignment");
+    if (Section->UseCodeAlign()) {
+      getStreamer().emitCodeAlignment(Alignment, /*MaxBytesToEmit=*/0);
+    } else {
+      // FIXME: Target specific behavior about how the "extra" bytes are filled.
+      getStreamer().emitValueToAlignment(Alignment, /*Value=*/0,
+                                         /*ValueSize=*/1,
+                                         /*MaxBytesToEmit=*/0);
+    }
+  } else {
+    // Align the next field of this struct
+    StructInfo &Structure = StructInProgress.back();
+    Structure.NextFieldOffset = llvm::alignTo(Structure.Size, Alignment);
+  }
+
+  return ReturnVal;
+}
+
 /// parseDirectiveAlign
 ///  ::= align expression
 bool MasmParser::parseDirectiveAlign() {
   SMLoc AlignmentLoc = getLexer().getLoc();
   int64_t Alignment;
 
-  if (checkForValidSection())
-    return addErrorSuffix(" in align directive");
   // Ignore empty 'align' directives.
   if (getTok().is(AsmToken::EndOfStatement)) {
     Warning(AlignmentLoc, "align directive with no operand is ignored");
     return parseToken(AsmToken::EndOfStatement);
   }
   if (parseAbsoluteExpression(Alignment) ||
       parseToken(AsmToken::EndOfStatement))
     return addErrorSuffix(" in align directive");
 
-  // Always emit an alignment here even if we thrown an error.
+  // Always emit an alignment here even if we throw an error.
   bool ReturnVal = false;
 
-  // Reject alignments that aren't either a power of two or zero, for gas
+  // Reject alignments that aren't either a power of two or zero, for ML.exe
   // compatibility. Alignment of zero is silently rounded up to one.
   if (Alignment == 0)
     Alignment = 1;
   if (!isPowerOf2_64(Alignment))
-    ReturnVal |= Error(AlignmentLoc, "alignment must be a power of 2");
+    ReturnVal |= Error(AlignmentLoc, "alignment must be a power of 2; was " +
+                                         std::to_string(Alignment));
 
-  // Check whether we should use optimal code alignment for this align
-  // directive.
-  const MCSection *Section = getStreamer().getCurrentSectionOnly();
-  assert(Section && "must have section to emit alignment");
-  if (Section->UseCodeAlign()) {
-    getStreamer().emitCodeAlignment(Alignment, /*MaxBytesToEmit=*/0);
-  } else {
-    // FIXME: Target specific behavior about how the "extra" bytes are filled.
-    getStreamer().emitValueToAlignment(Alignment, /*Value=*/0, /*ValueSize=*/1,
-                                       /*MaxBytesToEmit=*/0);
-  }
+  if (emitAlignTo(Alignment))
+    ReturnVal |= addErrorSuffix(" in align directive");
+
+  return ReturnVal;
 
+/// parseDirectiveEven
+///  ::= even
+bool MasmParser::parseDirectiveEven() {
+  if (parseToken(AsmToken::EndOfStatement))
+    return addErrorSuffix(" in even directive");
+
+  // Always emit an alignment here even if we throw an error.
+  bool ReturnVal = false;
+
+  if (emitAlignTo(2))
+    ReturnVal |= addErrorSuffix(" in even directive");
+
   return ReturnVal;
 }
 
 /// parseDirectiveFile
 /// ::= .file filename
 /// ::= .file number [directory] filename [md5 checksum] [source source-text]
 bool MasmParser::parseDirectiveFile(SMLoc DirectiveLoc) {
   // FIXME: I'm not sure what this is.
void MasmParser::initializeDirectiveKindMap() {
   DirectiveKindMap["sdword"] = DK_SDWORD;
   DirectiveKindMap["fword"] = DK_FWORD;
   DirectiveKindMap["qword"] = DK_QWORD;
   DirectiveKindMap["sqword"] = DK_SQWORD;
   DirectiveKindMap["real4"] = DK_REAL4;
   DirectiveKindMap["real8"] = DK_REAL8;
   DirectiveKindMap["real10"] = DK_REAL10;
   DirectiveKindMap["align"] = DK_ALIGN;
-  // DirectiveKindMap[".org"] = DK_ORG;
+  DirectiveKindMap["even"] = DK_EVEN;
+  DirectiveKindMap["org"] = DK_ORG;
   DirectiveKindMap["extern"] = DK_EXTERN;
   DirectiveKindMap["public"] = DK_PUBLIC;
   // DirectiveKindMap[".comm"] = DK_COMM;
   DirectiveKindMap["comment"] = DK_COMMENT;
   DirectiveKindMap["include"] = DK_INCLUDE;
   DirectiveKindMap["repeat"] = DK_REPEAT;
   DirectiveKindMap["rept"] = DK_REPEAT;
   DirectiveKindMap["while"] = DK_WHILE;

llvm/test/tools/llvm-ml/align_directives.asm

+; RUN: llvm-ml -filetype=s %s /Fo - | FileCheck %s
+
+.data
+
+align_test:
+ALIGN 16
+; CHECK-LABEL: align_test:
+; CHECK-NEXT: .p2align 4
+
+org_test:
+ORG 256
+; CHECK-LABEL: org_test:
+; CHECK-NEXT: .org 256, 0
+
+align_struct STRUCT
+  BYTE ?
+
+  ALIGN 4
+  x BYTE ?
+  x_succ BYTE ?
+  BYTE ?
+
+  ALIGN 2
+  y BYTE ?
+  y_succ BYTE ?
+
+  ALIGN 1
+  z BYTE ?
+
+  EVEN
+  q BYTE ?
+align_struct ENDS
+
+struct_align_data ALIGN_STRUCT <101, 102, 103, 104, 105, 106, 107, 108>
+; CHECK-LABEL: struct_align_data:
+; CHECK-NEXT: .byte 101
+; CHECK-NEXT: .zero 3
+; CHECK-NEXT: .byte 102
+; CHECK-NEXT: .byte 103
+; CHECK-NEXT: .byte 104
+; CHECK-NEXT: .zero 1
+; CHECK-NEXT: .byte 105
+; CHECK-NEXT: .byte 106
+; CHECK-NEXT: .byte 107
+; CHECK-NEXT: .zero 1
+; CHECK-NEXT: .byte 108
+
+org_struct STRUCT
+  x BYTE ?
+  x_succ BYTE ?
+  ORG 15
+  y BYTE ?
+  y_succ BYTE ?
+  ORG 2
+  z BYTE ?
+  z_succ BYTE ?
+org_struct ENDS
+
+.code
+
+struct_align_test PROC
+
+x_align_test:
+  MOV eax, align_struct.x
+  MOV eax, align_struct.x_succ
+; CHECK-LABEL: x_align_test:
+; CHECK-NEXT: mov eax, 4
+; CHECK-NEXT: mov eax, 5
+
+y_align_test:
+  MOV eax, align_struct.y
+  MOV eax, align_struct.y_succ
+; CHECK-LABEL: y_align_test:
+; CHECK-NEXT: mov eax, 8
+; CHECK-NEXT: mov eax, 9
+
+z_align_test:
+  MOV eax, align_struct.z
+; CHECK-LABEL: z_align_test:
+; CHECK-NEXT: mov eax, 10
+
+q_even_test:
+  MOV eax, align_struct.q
+; CHECK-LABEL: q_even_test:
+; CHECK-NEXT: mov eax, 12
+
+size_align_test:
+  MOV eax, SIZEOF(align_struct)
+; CHECK-LABEL: size_align_test:
+; CHECK-NEXT: mov eax, 13
+
+  ret
+struct_align_test ENDP
+
+struct_org_test PROC
+; CHECK-LABEL: struct_org_test:
+
+field_positions:
+  MOV eax, org_struct.x
+  MOV eax, org_struct.x_succ
+  MOV eax, org_struct.y
+  MOV eax, org_struct.y_succ
+  MOV eax, org_struct.z
+  MOV eax, org_struct.z_succ
+; CHECK-LABEL: field_positions:
+; CHECK-NEXT: mov eax, 0
+; CHECK-NEXT: mov eax, 1
+; CHECK-NEXT: mov eax, 15
+; CHECK-NEXT: mov eax, 16
+; CHECK-NEXT: mov eax, 2
+; CHECK-NEXT: mov eax, 3
+
+  ret
+struct_org_test ENDP
+
+end

llvm/test/tools/llvm-ml/align_errors.asm

+; RUN: not llvm-ml -filetype=s %s /Fo /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
+
+.data
+org_struct STRUCT
+  x BYTE ?
+  x_succ BYTE ?
+  ORG 15
+  y BYTE ?
+  y_succ BYTE ?
+
+; CHECK: :[[# @LINE + 1]]:7: error: expected non-negative value in struct's 'org' directive; was -4
+  ORG -4
+
+  z BYTE ?
+  z_succ BYTE ?
+org_struct ENDS
+
+; CHECK: :[[# @LINE + 1]]:16: error: cannot initialize a value of type 'org_struct'; 'org' was used in the type's declaration
+x org_struct <>
+
+end