Skip to content

Commit 8663132

Browse files
committed
fixed zp vars 0 initialization
1 parent e2901cc commit 8663132

File tree

10 files changed

+175
-24
lines changed

10 files changed

+175
-24
lines changed

codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramAndVarsGen.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,10 +328,9 @@ internal class ProgramAndVarsGen(
328328
if (initializers.isNotEmpty()) {
329329
asmgen.out("prog8_init_vars\t.block")
330330
initializers.forEach { assign ->
331-
if((assign.value as? PtNumber)?.number != 0.0 || allocator.isZpVar(assign.target.identifier!!.name))
331+
val constvalue = assign.value as? PtNumber
332+
if(constvalue==null || constvalue.number!=0.0 || allocator.isZpVar(assign.target.identifier!!.name))
332333
asmgen.translate(assign)
333-
else
334-
throw AssemblyError("non-zp variable should not be initialized to zero; it will be zeroed as part of BSS clear")
335334
// the other variables that should be set to zero are done so as part of the BSS section clear.
336335
}
337336
asmgen.out(" rts\n .bend")

codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class IRCodeGen(
7575
initsToRemove += block to initialization
7676
}
7777
is PtNumber -> {
78-
require(initValue.number!=0.0) { "variable should not be initialized with 0, it will already be zeroed as part of BSS clear, initializer=$initialization" }
78+
require(initValue.number!=0.0 || variable.zpwish!=ZeropageWish.NOT_IN_ZEROPAGE) {"non-zp variable should not be initialized with 0, it will already be zeroed as part of BSS clear, initializer=$initialization" }
7979
variable.setOnetimeInitNumeric(initValue.number)
8080
initsToRemove += block to initialization
8181
}

compiler/src/prog8/compiler/Compiler.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ private fun processAst(program: Program, errors: IErrorReporter, compilerOptions
468468
errors.report()
469469
program.constantFold(errors, compilerOptions)
470470
errors.report()
471-
program.reorderStatements(errors)
471+
program.reorderStatements(compilerOptions, errors)
472472
errors.report()
473473
program.desugaring(errors, compilerOptions)
474474
errors.report()

compiler/src/prog8/compiler/astprocessing/AstExtensions.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ internal fun Program.checkValid(errors: IErrorReporter, compilerOptions: Compila
2020
checker.visit(this)
2121
}
2222

23-
internal fun Program.reorderStatements(errors: IErrorReporter) {
24-
val reorder = StatementReorderer(this, errors)
23+
internal fun Program.reorderStatements(options: CompilationOptions, errors: IErrorReporter) {
24+
val reorder = StatementReorderer(this, options, errors)
2525
reorder.visit(this)
2626
if(errors.noErrors()) {
2727
reorder.applyModifications()

compiler/src/prog8/compiler/astprocessing/SimplifiedAstMaker.kt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,11 @@ class SimplifiedAstMaker(private val program: Program, private val errors: IErro
165165
}
166166
}
167167

168-
if(srcAssign.origin == AssignmentOrigin.VARINIT && srcAssign.parent is Block && srcAssign.value.constValue(program)?.number==0.0)
169-
throw FatalAstException("should not have a redundant block-level variable=0 assignment; it will be zeroed as part of BSS clear")
168+
if(srcAssign.origin == AssignmentOrigin.VARINIT && srcAssign.parent is Block && srcAssign.value.constValue(program)?.number==0.0) {
169+
val zeropages = srcAssign.target.targetIdentifiers().mapNotNull { it.targetVarDecl()?.zeropage }
170+
if(zeropages.any {it==ZeropageWish.NOT_IN_ZEROPAGE})
171+
throw FatalAstException("should not have a redundant block-level variable=0 assignment for a non-ZP variable; it will be zeroed as part of BSS clear")
172+
}
170173

171174
val assign = PtAssignment(srcAssign.position, srcAssign.origin==AssignmentOrigin.VARINIT)
172175
val multi = srcAssign.target.multi

compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,11 @@ import prog8.ast.expressions.*
55
import prog8.ast.statements.*
66
import prog8.ast.walk.AstWalker
77
import prog8.ast.walk.IAstModification
8-
import prog8.code.core.AssociativeOperators
9-
import prog8.code.core.BaseDataType
10-
import prog8.code.core.DataType
11-
import prog8.code.core.IErrorReporter
8+
import prog8.code.core.*
129

1310
internal class StatementReorderer(
1411
val program: Program,
12+
val options: CompilationOptions,
1513
val errors: IErrorReporter
1614
) : AstWalker() {
1715
// Reorders the statements in a way the compiler needs.
@@ -114,10 +112,17 @@ internal class StatementReorderer(
114112
}
115113

116114
private fun canSkipInitializationWith0(decl: VarDecl): Boolean {
117-
// if the variable is declared in a block, we can omit the init with 0 because
118-
// the variable will be initialized to zero when the BSS section is cleared as a whole.
119-
if(decl.parent is Block)
120-
return true
115+
if(decl.parent is Block) {
116+
// if the variable is declared in a block and is NOT in ZEROPAGE, we can omit the init with 0 because
117+
// the variable will be initialized to zero when the BSS section is cleared as a whole.
118+
if (decl.zeropage == ZeropageWish.NOT_IN_ZEROPAGE)
119+
return true
120+
121+
// block level zp var that is not in zeropage, doesn't have to be cleared (will be done as part of bss clear at startup)
122+
// note: subroutine level var HAS to be cleared because it needs to be zero at every subroutine call!
123+
if (decl.zeropage == ZeropageWish.DONTCARE && options.zeropage == ZeropageType.DONTUSE)
124+
return true
125+
}
121126

122127
// if there is an assignment to the variable below it (regular assign, or For loop),
123128
// and there is nothing important in between, we can skip the initialization.

compiler/test/codegeneration/TestVariables.kt

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import prog8.ast.statements.Assignment
1010
import prog8.ast.statements.AssignmentOrigin
1111
import prog8.ast.statements.ForLoop
1212
import prog8.ast.statements.VarDecl
13+
import prog8.code.ast.PtAssignment
14+
import prog8.code.ast.PtNumber
1315
import prog8.code.target.C64Target
16+
import prog8.code.target.Cx16Target
1417
import prog8tests.helpers.ErrorReporterForTests
1518
import prog8tests.helpers.compileText
1619

@@ -106,6 +109,7 @@ class TestVariables: FunSpec({
106109

107110
test("global var init with array lookup should sometimes be const") {
108111
val src="""
112+
%zeropage dontuse
109113
main {
110114
111115
bool[] barray = [true, false, true, false]
@@ -220,4 +224,85 @@ main {
220224
(st[5] as Assignment).target.identifier?.nameInSource shouldBe listOf("v1")
221225
(st[6] as Assignment).target.identifier?.nameInSource shouldBe listOf("v0")
222226
}
227+
228+
test("nondirty zp variables should be explicitly initialized to 0") {
229+
val src="""
230+
main {
231+
ubyte @shared @requirezp zpvar
232+
ubyte @shared @requirezp @dirty dirtyzpvar
233+
234+
sub start() {
235+
ubyte @shared @requirezp zpvar2
236+
ubyte @shared @requirezp @dirty dirtyzpvar2
237+
}
238+
}"""
239+
240+
val result = compileText(Cx16Target(), false, src, outputDir, writeAssembly = true)!!.codegenAst
241+
242+
val main = result!!.allBlocks().first { it.name=="p8b_main" }
243+
main.children.size shouldBe 4
244+
val zeroassignlobal = main.children.single { it is PtAssignment } as PtAssignment
245+
(zeroassignlobal.value as PtNumber).number shouldBe 0.0
246+
zeroassignlobal.target.identifier!!.name shouldBe "p8b_main.p8v_zpvar"
247+
248+
val st = result.entrypoint()!!.children
249+
st.size shouldBe 4
250+
val zeroassign = st.single { it is PtAssignment } as PtAssignment
251+
(zeroassign.value as PtNumber).number shouldBe 0.0
252+
zeroassign.target.identifier!!.name shouldBe "p8b_main.p8s_start.p8v_zpvar2"
253+
}
254+
255+
test("nondirty non zp variables in block scope should not be explicitly initialized to 0 (bss clear takes care of it)") {
256+
val src="""
257+
%zeropage dontuse
258+
259+
main {
260+
ubyte @shared v1
261+
ubyte @shared @dirty dv1
262+
sub start() {
263+
ubyte @shared v2
264+
ubyte @shared @dirty dv2
265+
}
266+
}"""
267+
268+
val result = compileText(Cx16Target(), false, src, outputDir, writeAssembly = true)!!.codegenAst
269+
270+
// block level should not be intialized to 0 (will be done by BSS clear)
271+
val main = result!!.allBlocks().first { it.name=="p8b_main" }
272+
main.children.size shouldBe 3
273+
main.children.any { it is PtAssignment } shouldBe false
274+
275+
// subroutine should be initialized to 0 because that needs to be done on every call to the subroutine
276+
val st = result.entrypoint()!!.children
277+
st.size shouldBe 4
278+
val zeroassign = st.single { it is PtAssignment } as PtAssignment
279+
(zeroassign.value as PtNumber).number shouldBe 0.0
280+
zeroassign.target.identifier!!.name shouldBe "p8b_main.p8s_start.p8v_v2"
281+
}
282+
283+
test("nondirty explicit non zp variables in block scope should not be explicitly initialized to 0 (bss clear takes care of it)") {
284+
val src="""
285+
main {
286+
ubyte @shared @nozp v1
287+
ubyte @shared @dirty @nozp dv1
288+
sub start() {
289+
ubyte @shared @nozp v2
290+
ubyte @shared @dirty @nozp dv2
291+
}
292+
}"""
293+
294+
val result = compileText(Cx16Target(), false, src, outputDir, writeAssembly = true)!!.codegenAst
295+
296+
// block level should not be intialized to 0 (will be done by BSS clear)
297+
val main = result!!.allBlocks().first { it.name=="p8b_main" }
298+
main.children.size shouldBe 3
299+
main.children.any { it is PtAssignment } shouldBe false
300+
301+
// subroutine should be initialized to 0 because that needs to be done on every call to the subroutine
302+
val st = result.entrypoint()!!.children
303+
st.size shouldBe 4
304+
val zeroassign = st.single { it is PtAssignment } as PtAssignment
305+
(zeroassign.value as PtNumber).number shouldBe 0.0
306+
zeroassign.target.identifier!!.name shouldBe "p8b_main.p8s_start.p8v_v2"
307+
}
223308
})

docs/source/todo.rst

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
TODO
22
====
33

4-
BUG: fix ZP variables no longer getting initialized to 0 (see test.p8)
4+
Since fixing the missing zp-var initialization, programs grew in size again (assem)
5+
Are there any redundant block-level variable initializations to 0 that we can remove in peephole optimization for example?
6+
57

68
STRUCTS: are being developed in their own separate branch for now, called "structs".
79
Idea is to make it feature complete in the IR/Virtual target, then merge it to master?, and then start building the 6502 code generation for it.

examples/test.p8

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,47 @@
44
%zpallowed 224,255
55

66
main {
7-
uword @shared @requirezp var1 = 0
8-
uword @shared @requirezp var2 = 0
9-
uword @shared @requirezp var3 = 0
10-
uword @shared @requirezp var4 = 0
11-
uword @shared @requirezp var5 = 0
7+
uword @shared @requirezp zpvar1
8+
uword @shared @requirezp zpvar2
9+
uword @shared @requirezp zpvar3
10+
uword @shared @requirezp zpvar4
11+
uword @shared @requirezp zpvar5
12+
uword @shared @requirezp @dirty dzpvar1
13+
uword @shared @requirezp @dirty dzpvar2
14+
uword @shared @requirezp @dirty dzpvar3
15+
uword @shared @requirezp @dirty dzpvar4
16+
uword @shared @requirezp @dirty dzpvar5
17+
uword @shared @nozp var1
18+
uword @shared @nozp var2
19+
uword @shared @nozp var3
20+
uword @shared @nozp var4
21+
uword @shared @nozp var5
22+
uword @shared @nozp @dirty dvar1
23+
uword @shared @nozp @dirty dvar2
24+
uword @shared @nozp @dirty dvar3
25+
uword @shared @nozp @dirty dvar4
26+
uword @shared @nozp @dirty dvar5
1227

1328
sub start() {
29+
txt.print("address start of zpvars: ")
30+
txt.print_uw(&zpvar1)
31+
txt.nl()
32+
txt.print("address start of normal vars: ")
33+
txt.print_uw(&var1)
34+
txt.nl()
35+
36+
txt.print("non-dirty zp should all be 0: ")
37+
txt.print_uw(zpvar1)
38+
txt.spc()
39+
txt.print_uw(zpvar2)
40+
txt.spc()
41+
txt.print_uw(zpvar3)
42+
txt.spc()
43+
txt.print_uw(zpvar4)
44+
txt.spc()
45+
txt.print_uw(zpvar5)
46+
txt.nl()
47+
txt.print("non-dirty should all be 0: ")
1448
txt.print_uw(var1)
1549
txt.spc()
1650
txt.print_uw(var2)
@@ -22,6 +56,29 @@ main {
2256
txt.print_uw(var5)
2357
txt.nl()
2458

59+
txt.print("dirty zp may be random: ")
60+
txt.print_uw(dzpvar1)
61+
txt.spc()
62+
txt.print_uw(dzpvar2)
63+
txt.spc()
64+
txt.print_uw(dzpvar3)
65+
txt.spc()
66+
txt.print_uw(dzpvar4)
67+
txt.spc()
68+
txt.print_uw(dzpvar5)
69+
txt.nl()
70+
txt.print("dirty may be random: ")
71+
txt.print_uw(dvar1)
72+
txt.spc()
73+
txt.print_uw(dvar2)
74+
txt.spc()
75+
txt.print_uw(dvar3)
76+
txt.spc()
77+
txt.print_uw(dvar4)
78+
txt.spc()
79+
txt.print_uw(dvar5)
80+
txt.nl()
81+
2582
repeat {}
2683
}
2784
}

simpleAst/src/prog8/code/SymbolTable.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ class StStaticVariable(name: String,
200200
// Certain codegens might want to put them back into the variable directly.
201201
// For strings and arrays this doesn't occur - these are always already specced at creation time.
202202

203-
require(number!=0.0) { "variable should not be initialized with 0, it will already be zeroed as part of BSS clear" }
203+
require(number!=0.0 || zpwish!=ZeropageWish.NOT_IN_ZEROPAGE) { "non-zp variable should not be initialized with 0, it will already be zeroed as part of BSS clear" }
204204
initializationNumericValue = number
205205
}
206206

0 commit comments

Comments
 (0)