wiz-lang / wiz

A high-level assembly language for writing homebrew software and games on retro console platforms.
http://wiz-lang.org/
Other
409 stars 40 forks source link

For loop countinng down to 0 doesn't work #130

Open veremenko-y opened 3 years ago

veremenko-y commented 3 years ago

The following code is from the examples in the README.md

// x will be 0 after the loop.
for x in 31 .. 0 by -1 {
    // ...
}

Output:

* wiz: version 0.1.2 (alpha)
>> Parsing...
>> Compiling...
src/bugtest.wiz:14: error: inequality comparison `!=` is not defined between provided operand types `u8` and `iexpr`
src/bugtest.wiz:14: error: `for` loop range of `31` ..  `0` by `-1` is not supported.
* wiz: failed with 2 error(s).

Notes

Original file to reproduce the issue bugtest.txt Build from b25ba2d90288fc8a380d99b3ad08a5b48caaa484 commit Build with wiz\wiz.exe src/bugtest.wiz -o bugtest.nes command

Bananattack commented 2 years ago

Just adding a few notes. It's been a while since implementing this, so trying to log a bit of things about behaviour as the bug is still open.

1) a for loop that counts down from 31 .. 1 by -1 works ok. 2) the loop is always done as a do .. while loop, and the range bounds are inclusive, so the value of the counter after the loop is "supposed to be" the value one-past-the-end of the range. 3) First, the increment/decrement is done, then the compare-and-branch, or just branch. 4) it doesn't have code to check the negative flag as a side-effect of the loop counter decrement. 5) it doesn't seem to be able to do a comparison instruction either, probably because at compile-time it uses a signed integers for the loop bound, and it doesn't cast the result to the type of the counter.

To be honest, I kind of want to improve range literals, so that excluding end bounds is possible, and translating code is easier. I also want to fix for loops to be more usable in general, there's already an issue to allow runtime ranges in for loops too. I'll have to see about that, but at the very least: a) the negative flag should be checked, and b) failing that, comparison for loop wraparound should be possible for countdown loops. c) decrement-and-branch instruction support should be possible for systems that have it such as the Z80.

Anyway, this is the code responsible for most of the logic around for loops, it could certainly be improved in terms of how it builds its loops: https://github.com/wiz-lang/wiz/blob/a174205e1694ab225c11813d3a3ab9e81742869d/src/wiz/compiler/compiler.cpp#L6598-L6748

shinymerlyn commented 2 years ago

A few other examples (beyond the initially reported bug) that would be useful for runtime for-loops. Already covered in @Bananattack's notes above I think, but these are some specific examples that are in the code I'm working on now. Technically they're a separate bug, but while on this broader subject -

Allow the initial value to be set by transfer from another register:

var clear_value: u8 in a = 0;
SomeVariableInABank = clear_value;

y = clear_value;
do {
    SomeArrayVariableInABank[y] = clear_value;
    ++y;
} while !zero;

rewritten:

var clear_value: u8 in a = 0;
SomeVariableInABank = clear_value;

for y in clear_value .. 255 {  // would emit a `tay` instead of `ldy #0`
    SomeArrayVariableInABank[y] = clear_value;
}

Allow the initial value for the loop counter register to be set some arbitrary number of lines earlier, with code in between:

y = 7;
// some other code here

do {
    SomeArrayVariableInABank[x] >>>#= 1;
    ++x;
    --y;
} while !zero;

rewritten:

y = 7;
// some other code here

for y in y .. 1 by -1 {  // would not emit a loop init instruction here
    SomeArrayVariableInABank[x] >>>#= 1;
    ++x;
}
veremenko-y commented 2 years ago

for y in clear_value .. 255 { // would emit a tay instead of ldy #0

Looks more like an optimizer bug, not the loop related. I'd recommend creating an issue for this.

for y in y .. 1 by -1 { // would not emit a loop init instruction here

This looks like correct behaviour, unless I'm misunderstanding. You ask compiler not initalize y and take it as is.

shinymerlyn commented 2 years ago

for y in clear_value .. 255 { // would emit a tay instead of ldy #0

Looks more like an optimizer bug, not the loop related. I'd recommend creating an issue for this.

I apologize. The comments aren't explained well enough. This isn't an optimizer problem. It is a missing feature in the compiler, that you can't use a variable as one of the loop bounds.

I want to write this, but the compiler won't let me:

a = 0;
// some code here that uses a
for y in a .. 255 { ... }

The compiler lets me write the following code, and I get for-loop syntax, and I get the right range, but emits more code than I'd prefer for this exact scenario:

a = 0;
// some code here that uses a
for y in 0 .. 255 { ... }

The problem related to an optimization, but it is a hand-optimization problem, not a compiler automated optimization problem.


for y in y .. 1 by -1 { // would not emit a loop init instruction here

This looks like correct behaviour, unless I'm misunderstanding. You ask compiler not initalize y and take it as is.

Yeah, again a case of my comments being unclear.

The compiler doesn't let me write this code either. I get this error.

test.wiz:775: error: range start must be a compile-time integer literal

I am replying to this thread because on the discord @Bananattack asked for some more explicit examples, related to their notes on all the types of runtime for loops people would like supported. So I'm adding some in my project that I currently have to work around with do/while loops.

undisbeliever commented 2 years ago

I've looked through my code and I could not find any for loops, all of my loops were do { } while... or while true { }.

Here are three examples of do ... while loops that I would like to see written using for statements (beyond the standard 0 .. 31 inclusive and 8 .. 1 by -1 inclusive loops).


A common looping pattern in my code is a backwards loop from the last-index to 0 inclusive. (NOTE: this pattern only works if the initial loop variable is <= 0x80 for 8 bit indexes and <= 0x8000 for 16 bit indexes).

    x = N_ENTITIES_IN_ROOM_DATA - 1;
    do {
        push8(x);

        entities.spawn_entity(a = room.entity_xPos[x],
                              a = room.entity_yPos[x],
                              a = room.entity_type[x],
                              y = room.entity_parameter[x]);

        x = pop8();

        x--;
    } while !negative;

I also have a lot of loops that increment or decrement by 2 to access word arrays and structure-of-word-arrays data.

    // Loop through the entity structure-of-word-arrays
    // Loop through LAST_INDEX to 0 inclusive, decrementing by 2
    yy = (N_ENTITIES - 1) * 2;
    ^do {
        a = entities.SoA.shadowSize[unaligned yy] as u8;

        // ...

        yy--;
        yy--;
    } while !negative;

Countdown loops using a zeropage variable.

    _columnsLeft = a = MAP_WIDTH;
    do {
        y = map[x];

        *(far &snes.ppu.vram_write_data_l) = a = tileset.bottomLeft_low[y];
        *(far &snes.ppu.vram_write_data_h) = a = tileset.bottomLeft_high[y];
        *(far &snes.ppu.vram_write_data_l) = a = tileset.bottomRight_low[y];
        *(far &snes.ppu.vram_write_data_h) = a = tileset.bottomRight_high[y];
        x++;

        _columnsLeft--;
    } while !zero;
veremenko-y commented 2 years ago

Yeah, again a case of my comments being unclear. I am replying to this thread because on the discord Bananattack asked for some more explicit examples

Got it, sorry about that, I did indeed misunderstood. Also I haven't been following up on the Discord lately.

Not sure what Bananattack wants, but if I were him, I'd like those to be separate feature requests on Github, it'd be easier to track down, and they could close it if needed too.