pulp-platform / hwpe-mac-engine

An example Hardware Processing Engine
Other
9 stars 14 forks source link

Error in vector stride? #2

Closed lstrz closed 6 years ago

lstrz commented 6 years ago

I'm trying out the HWME in scalar product mode and have started with some very simple examples. I am using the default microcode supplied in the pulp-rt-example/accelerators/hwme example.

Job dependent parameters are:

hwme_a_addr_set((unsigned int) a);
hwme_b_addr_set((unsigned int) b);
hwme_c_addr_set((unsigned int) c);
hwme_d_addr_set((unsigned int) d);
hwme_nb_iter_set(10);
hwme_len_iter_set(2 - 1 /* My vectors are of length 2. Have to offset by -1 if I understand things correctly. */);
hwme_vectstride_set(0 * sizeof(uint32_t));
hwme_shift_simplemul_set(hwme_shift_simplemul_value(10, 0));

All the results should be the same because 0 for vectstride means I'm always reading the same (first) vector from my a and b inputs. The a, b, c and d arrays are more than 300 bytes apart, so there's no overlap.

This is what the first 15 elements of a, b, c and d look like before and after running the HWME.

Before.
a:  1968 b:  4068 c:     0 d:     0
a:   741 b: -3079 c:     0 d:     0
a:  1968 b: -4369 c:     0 d:     0
a:   741 b: -2234 c:     0 d:     0
a:  1968 b: -2579 c:     0 d:     0
a:   741 b:  1415 c:     0 d:     0
a:  -222 b:  4068 c:     0 d:     0
a:   542 b: -3079 c:     0 d:     0
a:  -222 b: -4369 c:     0 d:     0
a:   542 b: -2234 c:     0 d:     0
a:  -222 b: -2579 c:     0 d:     0
a:   542 b:  1415 c:     0 d:     0
a:  -597 b:  4068 c:     0 d:     0
a:   664 b: -3079 c:     0 d:     0
a:  -597 b: -4369 c:     0 d:     0

After.
a:  1968 b:  4068 c:     0 d: -2229
a:   741 b: -3079 c:     0 d:  5590
a:  1968 b: -4369 c:     0 d: -2229
a:   741 b: -2234 c:     0 d:  5590
a:  1968 b: -2579 c:     0 d:  5590
a:   741 b:  1415 c:     0 d:  5590
a:  -222 b:  4068 c:     0 d: -2229
a:   542 b: -3079 c:     0 d:  5590
a:  -222 b: -4369 c:     0 d:  5590
a:   542 b: -2234 c:     0 d:  5590
a:  -222 b: -2579 c:     0 d:     0
a:   542 b:  1415 c:     0 d:     0
a:  -597 b:  4068 c:     0 d:     0
a:   664 b: -3079 c:     0 d:     0
a:  -597 b: -4369 c:     0 d:     0

The scalar product of the two vectors is 5590: (4068 1968 - 3079 741) / 1024 = 5590. What I don't understand is where does -2229 come from.

Did I misunderstand any of the configuration options?

lstrz commented 6 years ago

I've constructed a small example that demonstrates the weird behavior well. hwme.zip

Output is:

a:     2 b:     3 c:     0 d:    10
a:     2 b:     2 c:     0 d:    10
a:     2 b:     3 c:     0 d:    10
a:     2 b:     3 c:     0 d:    10
a:     2 b:     3 c:     0 d:     4
a:     2 b:     3 c:     0 d:    10
a:     2 b:     3 c:     0 d:    10

In the fifth row, the 4 should be a 10 instead.

FrancescoConti commented 6 years ago

Hi @lstrz, I will gladly check your example when possible. It is possible that some parts of the accelerator, especially the datapath, contain a few bugs -- this specific accelerator is provided as an example so it's not thoroughly tested. I am more confident for what concerns the hwpe-stream and hwpe-ctrl IPs, although the versions used here could be (very slightly) outdated.

I'll let you know.

lstrz commented 6 years ago

Sounds good! I wasn't sure if I was doing something wrong or not, so while at it, I posted the example for narrowing down potential bugs in IPs you care about, as I have little experience in digital design.

FrancescoConti commented 6 years ago

Ok I found and fixed the issue, which has always been there. One of the common cases (two simultaneous valid handshakes in different streams) was not properly checked in the datapath. It was not previously exposed (some other change in the platform must have brought it out by slightly changing timings), but actually it impacted also my own test. It is now fixed in commit 087a7f3 of the HWME module, which you should get if you update the platform IPs with update-ips.

I also took the occasion to update hwpe-stream and hwpe-ctrl. They are not involved in this bug, but the new version should be anyways more robust, I've been recently working on a lot of testing on these versions.

lstrz commented 6 years ago

Perfect. Thank you very much!