On Mon, May 21, 2012 at 12:00 AM, Julius Baxter <[email protected]> wrote:
> On Sun, May 20, 2012 at 11:41 PM, Julius Baxter <[email protected]> 
> wrote:
>> On Fri, May 4, 2012 at 6:56 PM, R. Diez <[email protected]> wrote:
>>> Hi all:
>>>
>>> I have this source line in my modified or1200 test suite:
>>>
>>>  // Subtract a large negative from a large positive number. Should set
>>>  // both the overflow and carry flags.
>>>  TEST_INST_FF_I32_I32 0, SPR_SR_CY | SPR_SR_OV, l.sub, 0x7fffffff,
>>> 0x80000000
>>>
>>> which translates into the following pseudo-instructions:
>>>
>>>  set carry flag
>>>  set overflow flag
>>>  l.sub 0x7fffffff, 0x80000000
>>>
>>> When run against or1ksim, it generates the following results:
>>>
>>>  0xffffffff, carry flag set, overflow flag set
>>>
>>> However, it looks different when run against ORPSoC V2's or1200 RTL model
>>> (simulated with Icarus Verilog):
>>>
>>>  0xffffffff, carry flag set, _NO_ overflow flag
>>>
>>> I'm having similar issues with the multiply instructions, many carry and
>>> overflow flags are different. I wonder if I've made a mistake somewhere. Or
>>> are we really so lucky that nobody has ever noticed?
>>
>> No, this could potentially be a genuine problem.
>>
>> The subtract test is far from exhaustive, and no doubt there's some holes 
>> there.
>>
>> I haven't investigated this one, but I suspect there's some overflow
>> detection logic going awry (converting signed 0x80000000 into positive
>> for the addition doesn't work correctly I suspect.
>>
>> I'll look into this though.
>>
>> What other suspect overflow results have you seen?
>>
>> Thanks
>>
>> Julius
>
>
> The following patch appears to make OV work for this particular case
> (test case SW patch also trailing):
>
> Index: rtl/verilog/or1200/or1200_alu.v
> ===================================================================
> --- rtl/verilog/or1200/or1200_alu.v     (revision 798)
> +++ rtl/verilog/or1200/or1200_alu.v     (working copy)
> @@ -169,6 +169,11 @@
>  assign {cy_sum, result_sum} = (a + b_mux) + carry_in;
>  // Numbers either both +ve and bit 31 of result set
>  assign ov_sum = ((!a[width-1] & !b_mux[width-1]) & result_sum[width-1]) |
> +`ifdef OR1200_IMPL_SUB
> +               // Subtract larger negative from smaller positive
> +               ((!a[width-1] & b_mux[width-1]) & result_sum[width-1] &
> +                alu_op==`OR1200_ALUOP_SUB) |
> +`endif
>  // or both -ve and bit 31 of result clear
>                ((a[width-1] & b_mux[width-1]) & !result_sum[width-1]);
>  assign result_and = a & b;
>
>
> Index: sw/tests/or1200/sim/or1200-ov.S
> ===================================================================
> --- sw/tests/or1200/sim/or1200-ov.S     (revision 798)
> +++ sw/tests/or1200/sim/or1200-ov.S     (working copy)
> @@ -265,6 +265,16 @@
>        l.add   r3, r0, r0      ;// Should clear overflow
>        l.nop   0x2
>        CHECK_OV_CLEAR
> +
> +       // Subtract the biggest negative number from the
> +       // biggest positive number.
> +       l.movhi r4,0x7fff
> +       l.ori   r4,r4,0xffff
> +       l.movhi r5,0x8000
> +       l.sub   r3,r4,r5
> +       l.nop   2
> +       CHECK_OV_SET
> +
>  #endif
>
>        l.movhi r4, 0x8000

I created a bug and applied these fixes:

http://bugzilla.opencores.org/bugzilla4/show_bug.cgi?id=91
_______________________________________________
OpenRISC mailing list
[email protected]
http://lists.openrisc.net/listinfo/openrisc

Reply via email to