justinmeiners/lc3-vm

ins<15> instantiation - Use of uninitalised variable.

mikey-b opened this issue · 1 comments

May I just say that the use of this template pattern to "build" common behaviour between instructions is beautiful. Its nice to know I can still learn new things.

Again, while looking at branches - I have discovered a bug. As op is known at compile time, we can also compute opbit, again allowing us to completely remove the branches. Optimisations can and should already do this for us. However, if we utilise constexpr we get better error reporting.

    constexpr uint16_t opbit = (1 << op);
    if constexpr (0x4EEE & opbit) { r0 = (instr >> 9) & 0x7; }
    ... etc ...

The error is.

(363): note: see reference to function template instantiation 'void ins<15>(const uint16_t)' being compiled
(272) : warning C4700: uninitialized local variable 'r1' used

Relating to absolute jumps? Which suggests to me the bit pattern for 15 has an issue?

You have precisely the idea of how it is supposed to work. I think it is pretty nice as well, and is probably closer to how hardware like this might be built.

This is definitely a bug. From what I found it looked like OP_JSR was actually the problem.

lc3-alt.cpp: In function ‘void ins(uint16_t) [with unsigned int op = 4]’:
lc3-alt.cpp:282:23: warning: ‘r1’ may be used uninitialized in this function [-Wmaybe-uninitialized]
             reg[R_PC] = reg[r1];

JSR has two modes, and it looks like neither of our programs use the second mode which is why we didn't notice it.

I like the constexpr and will add it to opbit to emphasize what should be going on.
I won't add it to the conditional, as that requires C++17, and requires some other changes for clang.

#32