From 5e46e8ade9b125f60eca332c1ac079b5197b7909 Mon Sep 17 00:00:00 2001 From: Karol Stasiak Date: Wed, 2 Sep 2020 00:44:24 +0200 Subject: [PATCH] Fix alignment of substructures --- CHANGELOG.md | 2 + src/main/scala/millfork/MathUtils.scala | 2 +- src/main/scala/millfork/env/Environment.scala | 65 ++++++++++++++++++- src/main/scala/millfork/env/Thing.scala | 33 ++++++---- .../scala/millfork/test/StructSuite.scala | 24 +++++++ 5 files changed, 113 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7a545d4..56bb53fe 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/src/main/scala/millfork/MathUtils.scala b/src/main/scala/millfork/MathUtils.scala index 946c0840..e6e380a0 100644 --- a/src/main/scala/millfork/MathUtils.scala +++ b/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 } diff --git a/src/main/scala/millfork/env/Environment.scala b/src/main/scala/millfork/env/Environment.scala index 2c741a68..418f2df8 100644 --- a/src/main/scala/millfork/env/Environment.scala +++ b/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 => diff --git a/src/main/scala/millfork/env/Thing.scala b/src/main/scala/millfork/env/Thing.scala index 896dc221..07362b93 100644 --- a/src/main/scala/millfork/env/Thing.scala +++ b/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 diff --git a/src/test/scala/millfork/test/StructSuite.scala b/src/test/scala/millfork/test/StructSuite.scala index e0279a81..5485c30c 100644 --- a/src/test/scala/millfork/test/StructSuite.scala +++ b/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) + } + } }