Optimization fail

[dadymax]

First of all I want to THANK YOU for mindcode! It's awesome! Especially in comparison with plain Mlog =)

I have fun with some straight usages of processors with using mindcode. But on some of my code I can compile mindcode to Mlog only with -O 0. With any other optimization value I have error like

314 instructions before optimizations.
Exception in thread "main" info.teksol.mc.mindcode.compiler.MindcodeInternalError: Internal error: compiler-generated variable '*tmp23' is uninitialized.
at info.teksol.mc.mindcode.compiler.optimization.DataFlowOptimizer.addUninitialized(DataFlowOptimizer.java:1023)

Example script with this issue may be found there
https://github.com/dadymax/mindustrylogicscripts/blob/4392b9607bbe656bf13cd382cb9d453ba7f0df88/stopProductionOnOverflow.mindcode

Compiled with -O 0 code work perfectly trought.

[cardillan]

Thanks for reporting the issue. I'm currently looking into it.

[cardillan]

@dadymax There's a workaround: replace this code

            if storage == null and block.@type in (@core-foundation, @core-shard, @core-nucleus) then
                storage = block;
                continue;
            end;

with

            if storage == null then
                if block.@type in (@core-foundation, @core-shard, @core-nucleus) then
                    storage = block;
                    continue;
                end;
            end;

The in list operator is quite new and apparently there's a problem with it. To avoid the bug, do not use it within more complex expressions (i.e., avoid using and or or with it) - that should generally help. I'm working on a fix.

[dadymax]

Understood. Thanks!

[cardillan]

@dadymax The fix has now been deployed.

Just a small performance tips on your script: it would be faster to have a single case expression over the block type in your code, and also to loop through linked blocks in reverse order (that way you don't need to check whether a given type of block has been already set; the first linked block of the given type will be processed last and remembered):

for var ind in 0 .. @links descending do
    var block = getlink(ind);

    case block.@type
        when @message then
            msg = block;

        when @titanium-conveyor, @armored-conveyor, @conveyor then
            // This preserves the order. It's a bit slower than the index-based access,
            // but for an array of 10 elements not by that much.
            convs[1 ... convsSize] = convs[0 ... convsSize - 1];
            convs[0] = block;
            convInd++;

        when @switch then
            resetSwitch = block;

        when @core-foundation, @core-shard, @core-nucleus then
            storage = block;

        when @memory-cell then
            memory2 = memory1;
            memory1 = block;
    end;
end;

Feel free to ask if there's something unclear.

Also, would it be OK if I link your repository in my Readme (the Mindcode syntax section)?

[dadymax]

Wow, nice trick! Order of convs not matter only order of memory blocks matter because of proxy direction. But now I think I want spend one memory cell position to save incremental "number of cell" and trought this order will be clear.
One more thing about speed: in this script part with getlink run only once on init state and then newer be called so I decide to not think about optimizations, Init may be slow by design =)

Feel free to ask if there's something unclear.

How this case block prevent reassign block of same type. I mean if link two messages then what actual message block will be stored in msg? Is getlink return in some guaranteed fixed order (message1, message2, ...)?

Also, would it be OK if I link your repository in my Readme (the Mindcode syntax section)?

Of course! I'll be glad if anything is useful in any way!

[cardillan]

You're right that the initialization doesn't need to be fast, but removing unneeded code (in this case, several instances checking that a variable is null) may also help make it smaller, which then leaves more space for the rest of the code to be optimized for speed.

(In this case the single case expression usually gets expanded to a jump table, but if space becomes tight, the optimizer should use a more compact version and optimize the rest of the code more. This can be helped by setting code weight locally, or even by setting optimization goal just for the Init function to size - i.e., #setlocal goal = size;.)

How this case block prevent reassign block of same type. I mean if link two messages then what actual message block will be stored in msg? Is getlink return in some guaranteed fixed order (message1, message2, ...)?

Generally, getlink returns blocks in the order in which they were linked. I'm not entirely sure what happens when you unlink a block and link it again, but it could be easily tested.

Your original code, when a block of a certain type was found, ignored the other blocks of the same type, so only the first one is remembered (I use this kind of initialization myself a lot). When iterating the links in the reversed order and storing e.g. each switch in the resetSwitch variable, this variable will eventually contain the first @switch block linked to the processor. If there are two switches, the second one gets overwritten by the first one in the loop.

When matching multiple blocks of the same type, e.g, memory cells or conveyors, I'm shifting them each time a new one is found. This is certainly faster/smaller for two variables (i.e., memory1/memory2). Using index-based access might be faster/smaller for conveyors, but for 10 of them, I'd still do it this way. For 15-20, I'd probably prefer index-based access as you did in your original script.