Issues :: Amber ARM-compatible core

I can confirm that the barrel shift bug is fixed. However i found more issues (in fairness i've got things setup to hunt for differences). I think this was introduced by recent changes.

The following code shows up another carry issue. Its odd in that removing cmp or tst shows up no differences.

    AREA    |main|, CODE, READONLY
    EXPORT  _start

ORG	&1000

_start

MOV	R0,#2
MOV	R1,#1
MOV	R3,#&930

CMP     R0,#1
MOV     R1,R3
TST     R1,#&10            ; =16
	
MOVCS   R0,#1 ; Expected
MOVCC   R0,#0 ; Actual

NOP
NOP

; I use this to stop simulations... ignore.	
SWI	&FFFFFF

NOP
NOP

END

The above was compiled with ASASM.

There is the output of the code running on my A540 with ARM3

https://www.dropbox.com/sc/h1emywrhd8m4wc2/AAAkJrP_0yuTb5tX39OWSouLa

Here is the output of the ArcEM vs Amber comparison tool.

stephen@development:~/svnwork/archie/bench/tests/fuzz$ ./program/Va23_core ../../tstbug2.bin Executing % on Amber/ArcEM Comparison 56 56 Failed. errors: expected,actual,bitwise difference r0(0x00000001,0x00000000,0x00000001) r15(0x24001030,0x04001033,0x20000003)

initial: r0=0x7e1d4e3c,r1=0x06f1d01b,r2=0x1d5d2ed9,r3=0x6f6f8292,r4=0x7c6b63c9,r5=0x2f69a363,r6=0x19430192,r7=0x0de580bf,r8=0x4c467585,r9=0x41ce04b8,r10=0x44a2b75b,r11=0x0b00c412,r12=0x3119e1f5,r13=0x6b052991,r14=0x66b52125,r15=0x569d67a8,r8_firq=0x304dc14d,r9_firq=0x611dec5a,r10_firq=0x1b088c0e,r11_firq=0x147c49d9,r12_firq=0x4ab42a1b,r13_firq=0x0e0912fe,r14_firq=0x464da53c,r13_irq=0x529e0ec9,r14_irq=0x4580ab03,r13_svc=0x5c49d017,r14_svc=0x322acccd,

arcem: r0=0x00000001,r1=0x00000930,r2=0x1d5d2ed9,r3=0x00000930,r4=0x7c6b63c9,r5=0x2f69a363,r6=0x19430192,r7=0x0de580bf,r8=0x4c467585,r9=0x41ce04b8,r10=0x44a2b75b,r11=0x0b00c412,r12=0x3119e1f5,r13=0x6b052991,r14=0x66b52125,r15=0x24001030,r8_firq=0x304dc14d,r9_firq=0x611dec5a,r10_firq=0x1b088c0e,r11_firq=0x147c49d9,r12_firq=0x4ab42a1b,r13_firq=0x0e0912fe,r14_firq=0x464da53c,r13_irq=0x529e0ec9,r14_irq=0x4580ab03,r13_svc=0x5c49d017,r14_svc=0x322acccd,

amber: r0=0x00000000,r1=0x00000930,r2=0x1d5d2ed9,r3=0x00000930,r4=0x7c6b63c9,r5=0x2f69a363,r6=0x19430192,r7=0x0de580bf,r8=0x4c467585,r9=0x41ce04b8,r10=0x44a2b75b,r11=0x0b00c412,r12=0x3119e1f5,r13=0x6b052991,r14=0x66b52125,r15=0x04001033,r8_firq=0x304dc14d,r9_firq=0x611dec5a,r10_firq=0x1b088c0e,r11_firq=0x147c49d9,r12_firq=0x4ab42a1b,r13_firq=0x0e0912fe,r14_firq=0x464da53c,r13_irq=0x529e0ec9,r14_irq=0x4580ab03,r13_svc=0x5c49d017,r14_svc=0x322acccd,

I've created a little resource for you. If you download this and compile it, in addition to the fuzzer, you get a little program that will execute a raw binary file @ 0x1000 in memory on both ArcEM and Amber simultaneously and give you a comparison of the result.

All you have to do ORG your program @ 0x1000 and you can terminate execution with SWI 0xFFFFFF.

The example above is provided in the bugs folder (although you may need to tweek it to use something other than asasm and the riscos objcopy).

To invoke it first "make" and then ./program/Va23_core <binary name>

Note it cannot be an elf it has to be raw. You also need verilator (i use debian testing version).

Not only does it do the comparison but it spits out a vcd and a disassembly of the execute path to amber.vcd and amber.dis.

Its pretty good at getting you to the heart of a problem fast. If want you can also follow the ArcEM execution path by turning DEBUG=yes for the arm project in the Make file.

Hopefully people will find this useful.

https://www.dropbox.com/s/dyhxewcyubyflfu/fuzz2.tgz?dl=0

Yuck opencores mangled that

to execute ./program/Va23_core programname

It looks like this was introduced by the fix for 2492 (commit 83) The carry rules are more complicated than the way they are currently implemented in Amber. Because Amber short circuits the barrel shifter for immediate operands it needs to calculate the correct carry when it does that. Currently it doesnt and hence the wrong carry results will be obtained.

I'll try and make it more clear... This code needs to give a carry result that gets picked up instead of barrel shift out if the operand is immediate.

assign imm32_nxt = // add 0 to Rm itype == MULT ? { 32'd0 } : // 4 x number of registers itype == MTRANS ? { mtrans_base_reg_change } : itype == BRANCH ? { offset24 } : itype == TRANS ? { offset12 } : instruction[11:8] == 4'h0 ? { 24'h0, imm8[7:0] } : instruction[11:8] == 4'h1 ? { imm8[1:0], 24'h0, imm8[7:2] } : instruction[11:8] == 4'h2 ? { imm8[3:0], 24'h0, imm8[7:4] } : instruction[11:8] == 4'h3 ? { imm8[5:0], 24'h0, imm8[7:6] } : instruction[11:8] == 4'h4 ? { imm8[7:0], 24'h0 } : instruction[11:8] == 4'h5 ? { 2'h0, imm8[7:0], 22'h0 } : instruction[11:8] == 4'h6 ? { 4'h0, imm8[7:0], 20'h0 } : instruction[11:8] == 4'h7 ? { 6'h0, imm8[7:0], 18'h0 } : instruction[11:8] == 4'h8 ? { 8'h0, imm8[7:0], 16'h0 } : instruction[11:8] == 4'h9 ? { 10'h0, imm8[7:0], 14'h0 } : instruction[11:8] == 4'ha ? { 12'h0, imm8[7:0], 12'h0 } : instruction[11:8] == 4'hb ? { 14'h0, imm8[7:0], 10'h0 } : instruction[11:8] == 4'hc ? { 16'h0, imm8[7:0], 8'h0 } : instruction[11:8] == 4'hd ? { 18'h0, imm8[7:0], 6'h0 } : instruction[11:8] == 4'he ? { 20'h0, imm8[7:0], 4'h0 } : { 22'h0, imm8[7:0], 2'h0 } ;

https://www.dropbox.com/s/2zvr5tu6lxl97mp/soak_tst.txt?dl=0

This highlights the issue with the barrel shifter short circuit. Every possible immediate is tested with 50 random register sets.

Looking at the fuzz output i think i can simplify this with this pseudo code...

If (operand is immediate) then

if (shift is zero)

   use carry in

else

   use high bit of imm32_nxt

end if

I fixed it for TST... what i did was i added this in a23_execute.v

assign barrel_shift_carry_real = i_barrel_shift_data_sel == 2'd0 ? (i_imm_shift_amount == 0 ? status_bits_flags1 : i_imm3231) : barrel_shift_carry;

I then changed the u_alu instance's i_barrel_shift_carry pin from

.i_barrel_shift_carry   ( barrel_shift_carry ),

to

.i_barrel_shift_carry   ( barrel_shift_carry_real ),

The soak test then passes.

Possibly a separate bug, but related... I found this in a23_decode.v

assign shift_imm = instruction11:7;

which is broken when the immediate is 0x80.

i changed it to...

assign shift_imm = {instruction11:8,1'b0};

With the two changes below the soak tests give...

https://www.dropbox.com/s/qtsf49ejbodu2wo/soak_pass.txt?dl=0

With the two changes below the soak tests give...

https://www.dropbox.com/s/qtsf49ejbodu2wo/soak_pass.txt?dl=0

A better fix is to not made the change to a23_decode and use this instead...

assign barrel_shift_carry_real = i_barrel_shift_data_sel == 2'd0 ? (i_imm_shift_amount4:1 == 0 ? status_bits_flags1 : i_imm3231) : barrel_shift_carry;

I fuzzed all ALU instructions with immediate operands. This change fixes all of them.

Does it fix the ANDS issue as well?

It fixes all the immediate ones. not the register <-> register ones. I'm working on those

The following instructions seem to need to use_carry_in...

EOR/TEQ, TST/AND, MOV, MVN

I've added carry ins to those guys and added the fix with the barrel shifter shortcut and things are looking much better again.

I also noticed a small problem with the LDM/STM fix. We need to not use the user registers on the second cycle ( control_state == MTRANS_EXEC1 ) because we end up writing back to the wrong base register if we do. So you just need to revert out the change on that cycle.

I'm putting together a base SVN clean patch with all the fixes in it.

https://www.dropbox.com/s/u5iovqsl2avhnda/amber23_fixes.diff?dl=0

This patch fixes all open amber23 bugs.

Fixes all the carry issues (verified with a fuzz - will post the fuzz result)
Fixes all the rrx issues.
Fixes the LDM/STM issue

Also patches the use of the wire/reg name type to itype. This is a reserved keyword on some verilog variations.

Note this patch doesnt include my Data abort fixes or the translate pin additions (Implemented as o_wb_tga).

Here are the fuzz results...

https://www.dropbox.com/s/nwjfok2fqzh0ut1/soak_patch.txt?dl=0

Summary....

Executed: 44124768 Skipped: 32 Passed: 44124768 Failed: 0

I wouldnt swear that all the bugs are gone but i'd say this was a big step forward in confidence.

With these fixes in (and some help with another part of my core from Till Harbaum) I was able to run Archimedes Elite nicely.

Code changes from Stephen added to release 87 for a23, and release 88 for a25. This fixes the issue.

Browse

Amber ARM-compatible core