Invalid asm in example for PIT in wiki?

[Techflash]

In the OSDev Wiki Page for the PIT, the code for the "TimerIRQ" part appears to be invalid. When I tried compiling it in nasm (to see it in the more familiar GAS syntax), it gives an error about the line:

Code: Select all

or eax, or eax ; quick way to compare to 0

Does anyone know if I'm missing something (wrong assembler maybe?) Or is this just supposed to be psuedocode or something?

[Octocontrabass]

It's a typo. It's supposed to be "or eax, eax".

There are other problems besides typos. The sleep function doesn't need to disable interrupts, and should use HLT or yield for other tasks to run instead of that incredibly wasteful NOP loop. (I'd argue it shouldn't be written in assembly in the first place - using assembly makes it more difficult to understand what the code is doing.)

[Techflash]

It's a typo. It's supposed to be "or eax, eax".

There are other problems besides typos. The sleep function doesn't need to disable interrupts, and should use HLT or yield for other tasks to run instead of that incredibly wasteful NOP loop. (I'd argue it shouldn't be written in assembly in the first place - using assembly makes it more difficult to understand what the code is doing.)

Alright, I'll see if it works.
I agree, however it being written in assembly does make it fast. Also I replaced the nops with the io wait assembly example (basically just send a 0 to io port 0x80 to waste a few microseconds)

[Octocontrabass]

it being written in assembly does make it fast

Modern compilers usually beat hand-written assembly for speed, and they let you optimize for different CPUs without changing your code.

Even in this example code there's obvious room for improvement: the "or eax, eax" instruction causes a false dependency.

Also I replaced the nops with the io wait assembly example

That still doesn't wait for long enough. You should use HLT instead.