On Sat, Jun 2, 2012 at 4:35 PM, Julius Baxter <[email protected]> wrote:
> On Tue, May 29, 2012 at 7:02 PM, R. Diez <[email protected]> wrote:
>> Hallo Julius:
>>
>>> [...]
>>
>>> I created a bug and applied these fixes:
>>> http://bugzilla.opencores.org/bugzilla4/show_bug.cgi?id=91
>>
>>
>>
>> I have re-run my modified test suite once more against the patched ORPSoC
>> V2, and a different test fails now. That test used to succeed before the
>> patch. It's the second test in asm-sub.S :
>>
>> // Subtract two small positive numbers. Sets the carry, but never the
>> overflow if the result is negative.
>>
>> OK -> TEST_INST_FF_I32_I32 0, SPR_SR_CY | SPR_SR_OV, l.sub, 0x00000003,
>> 0x00000002
>>
>> fails -> TEST_INST_FF_I32_I32 0, SPR_SR_CY | SPR_SR_OV, l.sub, 0x00000001,
>> 0x00000002
>>
>>
>> According to the test case (and to or1ksim's results), the overflow flag
>> should _not_ be set in the case, but it is with the new ORPSoC V2.
>
> OK, I've looked into this. I certainly think that last fix was not sufficient.
>
> I have expanded the ORPSoC overflow flag testsuite and got a better fix.
>
> I've marked the bug as open again, too.
>
> If I submit an RTL patch, are you happy to test it against your suite
> before I check this in?
>
> Thanks,
>
> Julius
OK, I think I have identified all of the cases where we should get
overflow, and tested them and made the RTL behave appropriately.
I think the cases for add and subtract are:
a) both numbers positive, result is two's complement negative (bit 31 set)
b) both numbers are two's complement negative and the result is
positive (bit 31 clear)
in for subtract, it's:
a) subtract positive from negative and result is positive (bit 31 clear)
b) subtract the largest 32-bit two's complement negative number from 0
(unable to represent that as a positive two's complement number)
The following patch should fix this in the OR1200:
Index: rtl/verilog/or1200/or1200_alu.v
===================================================================
--- rtl/verilog/or1200/or1200_alu.v (revision 809)
+++ rtl/verilog/or1200/or1200_alu.v (working copy)
@@ -167,15 +167,22 @@
`endif
`endif
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]) |
+
+assign ov_sum =
+ // When adding, numbers both +ve and result is negative,
+ // or, both -ve and bit 31 of result clear
+ ((a[width-1] == b[width-1]) &
+ (a[width-1] ^ result_sum[width-1]) &
+ alu_op==`OR1200_ALUOP_ADD)
`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) |
+ // Subtract positive from a negative and got positive result
+ | ((((a[width-1] & !b[width-1]) & !result_sum[width-1]) |
+ // Or, annoying case of subtracting -INT_MAX from zero
+ (!(|a) & (b[width-1] & !(|b[width-2:0])))) &
+ 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;
//
_______________________________________________
OpenRISC mailing list
[email protected]
http://lists.openrisc.net/listinfo/openrisc
