[parc][amo] Fix amo.min by making the comparison signed

[hawajkm]

The amo.min implementation assumes unsigned min function. To fix this, the values are converted to signed to make python perform a signed minimum.

[cbatten]

Hmm ... well this depends on the ISA right? Other apps might rely on amo.min being unsigned? Looks like RISC-V supports both AMOMIN (signed) and AMOMINU (unsigned).

[hawajkm]

Correct.

The assumption seems like that amo.min is signed. The current pydgin code-base fails for bthread tests. Upon further inspection and chatting with @ss2783, I saw that the faliure comes from this:

bthread-BasicUtils-utst

As you can see from the unit test highlighted above, the unit test assumes signed comparison. Also, old maven-isa-sim seems to implement it that way, too:

maven-isa-sim:amo.min

I think it makes sense to implement both signed and unsigned variants, but I guess the one we have now should be signed as per convention and legacy implementation.

-Khalid

[hawajkm]

I noticed that the amo.min is still failing. After further inspection, I see that python treats signed and unsigned number strictly differently. To make sure that the memory is not corrupted with signed numbers, I have added an unsigned function to python.utils! This unsigned function will be used by amo.min to convert the outcome of the comparison to unsigned again.

Currently, the only test that fails is syscall, due to syscall not implemented.

parcv1-addiu.out: [ passed ] ./parcv1-addiu parcv1-ori.out: [ passed ] ./parcv1-ori parcv1-lui.out: [ passed ] ./parcv1-lui parcv1-bne.out: [ passed ] ./parcv1-bne parcv1-addu.out: [ passed ] ./parcv1-addu parcv1-lw.out: [ passed ] ./parcv1-lw parcv1-sw.out: [ passed ] ./parcv1-sw parcv1-jal.out: [ passed ] ./parcv1-jal parcv1-jr.out: [ passed ] ./parcv1-jr parcv2-andi.out: [ passed ] ./parcv2-andi parcv2-xori.out: [ passed ] ./parcv2-xori parcv2-slti.out: [ passed ] ./parcv2-slti parcv2-sltiu.out: [ passed ] ./parcv2-sltiu parcv2-sll.out: [ passed ] ./parcv2-sll parcv2-srl.out: [ passed ] ./parcv2-srl parcv2-sra.out: [ passed ] ./parcv2-sra parcv2-subu.out: [ passed ] ./parcv2-subu parcv2-and.out: [ passed ] ./parcv2-and parcv2-or.out: [ passed ] ./parcv2-or parcv2-xor.out: [ passed ] ./parcv2-xor parcv2-nor.out: [ passed ] ./parcv2-nor parcv2-slt.out: [ passed ] ./parcv2-slt parcv2-sltu.out: [ passed ] ./parcv2-sltu parcv2-sllv.out: [ passed ] ./parcv2-sllv parcv2-srlv.out: [ passed ] ./parcv2-srlv parcv2-srav.out: [ passed ] ./parcv2-srav parcv2-lh.out: [ passed ] ./parcv2-lh parcv2-lhu.out: [ passed ] ./parcv2-lhu parcv2-lb.out: [ passed ] ./parcv2-lb parcv2-lbu.out: [ passed ] ./parcv2-lbu parcv2-sh.out: [ passed ] ./parcv2-sh parcv2-sb.out: [ passed ] ./parcv2-sb parcv2-beq.out: [ passed ] ./parcv2-beq parcv2-blez.out: [ passed ] ./parcv2-blez parcv2-bgez.out: [ passed ] ./parcv2-bgez parcv2-bltz.out: [ passed ] ./parcv2-bltz parcv2-bgtz.out: [ passed ] ./parcv2-bgtz parcv2-j.out: [ passed ] ./parcv2-j parcv2-jalr.out: [ passed ] ./parcv2-jalr parcv2-mul.out: [ passed ] ./parcv2-mul parcv2-div.out: [ passed ] ./parcv2-div parcv2-divu.out: [ passed ] ./parcv2-divu parcv2-rem.out: [ passed ] ./parcv2-rem parcv2-remu.out: [ passed ] ./parcv2-remu parcv2-mfc0.out: [ passed ] ./parcv2-mfc0

[berkinilbeyi]

It's not Python, but RPython that differentiates signed and unsigned numbers. The current convention in Pydgin is to store only unsigned values in memory. The way you initialize unsigned values in RPython is using r_uint( value ). The way you currently implemented unsigned is incorrect because it will still return you a signed type, so this will likely fail in Travis CI again. In fact, you don't need to define a new function in util.py; trim_32 does what you want. That function implies unsigning the argument. So all you need should be s.mem.write( s.rf[inst.rs], 4, trim_32( min( signed( temp ), signed( s.rf[inst.rt] ) ) ) ). It would also be helpful to look at how RISC-V defines amomin.w (https://github.com/cornell-brg/pydgin/blob/master/riscv/isa_RV32A.py#L84-L90) vs amominu.w (https://github.com/cornell-brg/pydgin/blob/master/riscv/isa_RV32A.py#L100-L106). Note that RISC-V has both 32-bit and 64-bit variants of these instructions, so the trim function is refactored to be more generic than just 32-bit types.

[hawajkm]

Hrmmm ... Okay. I will amend the highlighted issues and push again. But, it seems it is passing Travis CI :).

[hawajkm]

Okay! I have changed it. It is passing all tests.

Again, syscall is still failing due to unimplemented system calls

parcv1-addiu.out: [ passed ] ./parcv1-addiu parcv1-ori.out: [ passed ] ./parcv1-ori parcv1-lui.out: [ passed ] ./parcv1-lui parcv1-bne.out: [ passed ] ./parcv1-bne parcv1-addu.out: [ passed ] ./parcv1-addu parcv1-lw.out: [ passed ] ./parcv1-lw parcv1-sw.out: [ passed ] ./parcv1-sw parcv1-jal.out: [ passed ] ./parcv1-jal parcv1-jr.out: [ passed ] ./parcv1-jr parcv2-andi.out: [ passed ] ./parcv2-andi parcv2-xori.out: [ passed ] ./parcv2-xori parcv2-slti.out: [ passed ] ./parcv2-slti parcv2-sltiu.out: [ passed ] ./parcv2-sltiu parcv2-sll.out: [ passed ] ./parcv2-sll parcv2-srl.out: [ passed ] ./parcv2-srl parcv2-sra.out: [ passed ] ./parcv2-sra parcv2-subu.out: [ passed ] ./parcv2-subu parcv2-and.out: [ passed ] ./parcv2-and parcv2-or.out: [ passed ] ./parcv2-or parcv2-xor.out: [ passed ] ./parcv2-xor parcv2-nor.out: [ passed ] ./parcv2-nor parcv2-slt.out: [ passed ] ./parcv2-slt parcv2-sltu.out: [ passed ] ./parcv2-sltu parcv2-sllv.out: [ passed ] ./parcv2-sllv parcv2-srlv.out: [ passed ] ./parcv2-srlv parcv2-srav.out: [ passed ] ./parcv2-srav parcv2-lh.out: [ passed ] ./parcv2-lh parcv2-lhu.out: [ passed ] ./parcv2-lhu parcv2-lb.out: [ passed ] ./parcv2-lb parcv2-lbu.out: [ passed ] ./parcv2-lbu parcv2-sh.out: [ passed ] ./parcv2-sh parcv2-sb.out: [ passed ] ./parcv2-sb parcv2-beq.out: [ passed ] ./parcv2-beq parcv2-blez.out: [ passed ] ./parcv2-blez parcv2-bgez.out: [ passed ] ./parcv2-bgez parcv2-bltz.out: [ passed ] ./parcv2-bltz parcv2-bgtz.out: [ passed ] ./parcv2-bgtz parcv2-j.out: [ passed ] ./parcv2-j parcv2-jalr.out: [ passed ] ./parcv2-jalr parcv2-mul.out: [ passed ] ./parcv2-mul parcv2-div.out: [ passed ] ./parcv2-div parcv2-divu.out: [ passed ] ./parcv2-divu parcv2-rem.out: [ passed ] ./parcv2-rem parcv2-remu.out: [ passed ] ./parcv2-remu parcv2-mfc0.out: [ passed ] ./parcv2-mfc0 parcv3-syscall.out: [ FAILED ] ./parcv3-syscall (line 26) parcv3-amo-add.out: [ passed ] ./parcv3-amo-add parcv3-amo-and.out: [ passed ] ./parcv3-amo-and parcv3-amo-or.out: [ passed ] ./parcv3-amo-or parcv3-amo-xchg.out: [ passed ] ./parcv3-amo-xchg parcv3-amo-min.out: [ passed ] ./parcv3-amo-min parcv3-movn.out: [ passed ] ./parcv3-movn parcv3-movz.out: [ passed ] ./parcv3-movz parcv3-add-s.out: [ passed ] ./parcv3-add-s parcv3-sub-s.out: [ passed ] ./parcv3-sub-s parcv3-mul-s.out: [ passed ] ./parcv3-mul-s parcv3-div-s.out: [ passed ] ./parcv3-div-s parcv3-c-eq-s.out: [ passed ] ./parcv3-c-eq-s parcv3-c-lt-s.out: [ passed ] ./parcv3-c-lt-s parcv3-c-le-s.out: [ passed ] ./parcv3-c-le-s parcv3-cvt-w-s.out: [ passed ] ./parcv3-cvt-w-s parcv3-cvt-s-w.out: [ passed ] ./parcv3-cvt-s-w

[berkinilbeyi]

Yep, I already merged it!

I think the syscall test is failing because Pydgin uses syscall emulation. That test might pass if you're using the pkernel (in the multicore branch) but I'm not sure.

[hawajkm]

I will look into the syscall-thing later. Thanks.