Idein / py-videocore6

Python library for GPGPU programming on Raspberry Pi 4
https://idein.jp
GNU General Public License v2.0
244 stars 28 forks source link

test_signal.py: Rotate test always selects the mul alu rotate #45

Closed wimrijnders closed 3 years ago

wimrijnders commented 3 years ago

Hi there,

I believe the following is incorrect:

Source:

        if i % 1 == 0:
            rotate(r1, r0, i)       # add alias
        else:
            nop().rotate(r1, r0, i) # mul alias

i % 1 will always return 1, therefore the condition is always false and the mul alias branch is always selected. I think it should be i % 2, so that add alias and mul alias are alternately outputted, which seems to be the intent here.

Do you agree?

wimrijnders commented 3 years ago

Actually, I believe it would be better to have separate loops for the add alu and mul alu, so that the test is complete. Same goes for the next loop, where the rotate value comes from register r5 instead of a small immediate.

notogawa commented 3 years ago

Thank you for your reporting. We will fix it.

wimrijnders commented 3 years ago

My pleasure!

wimrijnders commented 3 years ago

That was quick! Pulling and testing right away.

wimrijnders commented 3 years ago

Working!

However, I see something interesting: the assembled code output for rotate(r1, r0, i) (add alias) is the same as nop().rotate(r1, r0, i) (mul alias). In both cases, the mul ALU is used.

Perhaps it is the case that rotate only works via the mul ALU?

Terminus-IMRC commented 3 years ago

Yes, rotate only works on mul ALU as in VC4 QPU.

wimrijnders commented 3 years ago

Thank you for the confirmation! And thank you for the extremely quick fix.