Fix alignment of substructures

CHANGELOG.md

@@ -14,6 +14,8 @@
 
 * Fixed evaluation of division of large constants.
 
+* Fix: Structure alignment is now respected for substructures.
+
 * X16: Fixed the address of `vera_dc_hscale_hstop` register (#54) (thanks to @Kobrasadetin).
 
 * Fixed evaluation of more complex boolean expressions (#56).

src/main/scala/millfork/MathUtils.scala

@@ -10,6 +10,6 @@ object MathUtils {
     if (b == 0) a else gcd(b, a % b)
 
   def lcm(a: Int, b: Int): Int =
-    (a.toLong & b.toLong / gcd(a, b)).toInt
+    (a.toLong * b.toLong / gcd(a, b)).toInt
 
 }

src/main/scala/millfork/env/Environment.scala

@@ -1086,6 +1086,24 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
     addThing(UnionType(stmt.name, stmt.fields, stmt.alignment.getOrElse(NoAlignment)), stmt.position)
   }
 
+  def getTypeAlignment(t: VariableType, path: Set[String]): MemoryAlignment = {
+    val name = t.name
+    if (path.contains(name)) return null
+    t match {
+      case s: CompoundVariableType =>
+        if (s.mutableAlignment ne null) return s.mutableAlignment
+        var alignment = s.baseAlignment
+        for( ResolvedFieldDesc(fieldType, _, _) <- s.mutableFieldsWithTypes) {
+          val a = getTypeAlignment(fieldType, path + name)
+          if (a eq null) return null
+          alignment = alignment & a
+        }
+        s.mutableAlignment = alignment
+        alignment
+      case _ => t.alignment
+    }
+  }
+
   def getTypeSize(t: VariableType, path: Set[String]): Int = {
     val name = t.name
     if (path.contains(name)) return -1
@@ -1098,7 +1116,9 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
           for( ResolvedFieldDesc(fieldType, _, count) <- s.mutableFieldsWithTypes) {
             val fieldSize = getTypeSize(fieldType, newPath) * count.getOrElse(1)
             if (fieldSize < 0) return -1
+            sum = fieldType.alignment.roundSizeUp(sum)
             sum += fieldSize
+            sum = fieldType.alignment.roundSizeUp(sum)
           }
           s.mutableSize = sum
           if (sum > 0xff) {
@@ -1107,8 +1127,10 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
           val b = get[Type]("byte")
           var offset = 0
           for( ResolvedFieldDesc(fieldType, fieldName, count) <- s.mutableFieldsWithTypes) {
+            offset = fieldType.alignment.roundSizeUp(offset)
             addThing(ConstantThing(s"$name.$fieldName.offset", NumericConstant(offset, 1), b), None)
             offset += getTypeSize(fieldType, newPath) * count.getOrElse(1)
+            offset = fieldType.alignment.roundSizeUp(offset)
           }
           sum
         }
@@ -2120,6 +2142,7 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
         val builder = new ListBuffer[Subvariable]
         var offset = 0
         for(ResolvedFieldDesc(typ, fieldName, arraySize) <- s.mutableFieldsWithTypes) {
+          offset = getTypeAlignment(typ, Set()).roundSizeUp(offset)
           arraySize match {
             case None =>
               val suffix = "." + fieldName
@@ -2133,6 +2156,7 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
               // TODO
           }
           offset += typ.size * arraySize.getOrElse(1)
+          offset = getTypeAlignment(typ, Set()).roundSizeUp(offset)
         }
         builder.toList
       case s: UnionType =>
@@ -2225,8 +2249,35 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
     aliasesToAdd.foreach(a => things += a.name -> a)
   }
 
+  def fixStructAlignments(): Unit = {
+    val allStructTypes: Iterable[CompoundVariableType] = things.values.flatMap {
+      case s@StructType(name, _, _) => Some(s)
+      case s@UnionType(name, _, _) => Some(s)
+      case _ => None
+    }
+    for (t <- allStructTypes) {
+      t.baseAlignment match {
+        case DivisibleAlignment(n) if n < 1 =>
+          log.error(s"Type ${t.name} has invalid alignment ${t.alignment}")
+        case WithinPageAlignment =>
+          log.error(s"Type ${t.name} has invalid alignment ${t.alignment}")
+        case _ =>
+      }
+    }
+    var iterations = allStructTypes.size
+    while (iterations >= 0) {
+      var ok = true
+      for (t <- allStructTypes) {
+        if (getTypeAlignment(t, Set()) eq null) ok = false
+      }
+      if (ok) return
+      iterations -= 1
+    }
+    log.error("Cycles in struct definitions found")
+  }
+
   def fixStructSizes(): Unit = {
-    val allStructTypes: Iterable[VariableType] = things.values.flatMap {
+    val allStructTypes: Iterable[CompoundVariableType] = things.values.flatMap {
       case s@StructType(name, _, _) => Some(s)
       case s@UnionType(name, _, _) => Some(s)
       case _ => None
@@ -2243,6 +2294,16 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
     log.error("Cycles in struct definitions found")
   }
 
+  def fixAlignedSizes(): Unit = {
+    val allTypes: Iterable[VariableType] = things.values.flatMap {
+      case s:VariableType => Some(s)
+      case _ => None
+    }
+    for (t <- allTypes) {
+      t.alignedSize = getTypeAlignment(t, Set()).roundSizeUp(getTypeSize(t, Set()))
+    }
+  }
+
   def fixStructFields(): Unit = {
     // TODO: handle arrays?
     things.values.foreach {
@@ -2293,7 +2354,9 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
       case _ =>
     }
     fixStructFields()
+    fixStructAlignments()
     fixStructSizes()
+    fixAlignedSizes()
     val pointies = collectPointies(program.declarations)
     pointiesUsed("") = pointies
     program.declarations.foreach { decl =>

src/main/scala/millfork/env/Thing.scala

@@ -24,10 +24,10 @@ sealed trait Type extends CallableThing {
 
   def size: Int
 
-  def alignedSize: Int = alignment.roundSizeUp(size)
-
   def alignment: MemoryAlignment
 
+  def alignedSize: Int
+
   def isSigned: Boolean
 
   def isBoollike: Boolean = false
@@ -49,13 +49,19 @@ sealed trait Type extends CallableThing {
   def pointerTargetName: String = "byte"
 }
 
-sealed trait VariableType extends Type
+sealed trait VariableType extends Type {
+
+  var alignedSize: Int = if (alignment ne null) alignment.roundSizeUp(size) else size
+
+}
 
 case class Subvariable(suffix: String, offset: Int, typ: VariableType, arraySize: Option[Int] = None)
 
 case object VoidType extends Type {
   def size = 0
 
+  def alignedSize = 0
+
   def isSigned = false
 
   override def name = "void"
@@ -137,20 +143,23 @@ case class EnumType(name: String, count: Option[Int]) extends VariableType {
   override def alignment: MemoryAlignment = NoAlignment
 }
 
-sealed trait CompoundVariableType extends VariableType
-
-case class StructType(name: String, fields: List[FieldDesc], override val alignment: MemoryAlignment) extends CompoundVariableType {
+sealed trait CompoundVariableType extends VariableType {
   override def size: Int = mutableSize
   var mutableSize: Int = -1
+
   var mutableFieldsWithTypes: List[ResolvedFieldDesc] = Nil
+
+  override def alignment: MemoryAlignment = mutableAlignment
+  def baseAlignment: MemoryAlignment
+  //noinspection ConvertNullInitializerToUnderscore
+  var mutableAlignment: MemoryAlignment = null
   override def isSigned: Boolean = false
 }
 
-case class UnionType(name: String, fields: List[FieldDesc], override val alignment: MemoryAlignment) extends CompoundVariableType {
-  override def size: Int = mutableSize
-  var mutableSize: Int = -1
-  var mutableFieldsWithTypes: List[ResolvedFieldDesc] = Nil
-  override def isSigned: Boolean = false
+case class StructType(name: String, fields: List[FieldDesc], baseAlignment: MemoryAlignment) extends CompoundVariableType {
+}
+
+case class UnionType(name: String, fields: List[FieldDesc], baseAlignment: MemoryAlignment) extends CompoundVariableType {
 }
 
 case object FatBooleanType extends VariableType {
@@ -176,6 +185,8 @@ case object FatBooleanType extends VariableType {
 sealed trait BooleanType extends Type {
   def size = 0
 
+  def alignedSize = 0
+
   def isSigned = false
 
   override def isBoollike: Boolean = true

src/test/scala/millfork/test/StructSuite.scala

@@ -202,4 +202,28 @@ class StructSuite extends FunSuite with Matchers {
       m.readByte(0xc005) should equal(1)
     }
   }
+
+  test("Field alignment") {
+    val code =
+      """
+        |
+        |struct alignedbyte align(2) { byte x }
+        |struct S {
+        |    alignedbyte a,
+        |    alignedbyte b
+        |}
+        |
+        |byte offset_a @$c000
+        |byte offset_b @$c001
+        |
+        |void main() {
+        |    offset_a = S.a.offset
+        |    offset_b = S.b.offset
+        |}
+        |""".stripMargin
+    EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8086, Cpu.Motorola6809)(code){ m =>
+      m.readByte(0xc000) should equal(0)
+      m.readByte(0xc001) should equal(2)
+    }
+  }
 }