Move gemmini code under platform

[yamaguchi1024]

No description provided.

[codecov]

Codecov Report

Merging #180 (58917ff) into master (d1a217f) will decrease coverage by 1.78%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
- Coverage   81.77%   79.99%   -1.79%     
==========================================
  Files          33       32       -1     
  Lines        3298     2854     -444     
==========================================
- Hits         2697     2283     -414     
+ Misses        601      571      -30     

Impacted Files	Coverage Δ
tests/gemmini/harness_gemmini.py	62.96% <ø> (-0.28%)	⬇️
tests/gemmini/conv/test_gemmini_conv.py	3.93% <100.00%> (ø)
tests/gemmini/conv/test_gemmini_conv_ae.py	100.00% <100.00%> (ø)
tests/gemmini/conv/test_gemmini_conv_max_pool.py	12.82% <100.00%> (ø)
tests/gemmini/conv/test_gemmini_conv_no_pad.py	100.00% <100.00%> (ø)
tests/gemmini/conv/test_gemmini_conv_stride1.py	47.02% <100.00%> (ø)
tests/gemmini/conv/test_gemmini_conv_stride2.py	7.46% <100.00%> (ø)
tests/gemmini/matmul/test_gemmini_matmul_ae.py	100.00% <100.00%> (ø)
tests/gemmini/matmul/test_gemmini_matmul_paper.py	100.00% <100.00%> (ø)
tests/gemmini/matmul/test_gemmini_resnet_matmul.py	100.00% <100.00%> (ø)
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1a217f...58917ff. Read the comment docs.

[alexreinking]

I don't think harness_gemmini.py should be moved into the main sources? It certainly can't be right now since it imports pytest and that's not listed in the package requirements, so doing this would break the package.

[yamaguchi1024]

Removed the pytest import as it was not used anywhere. I'm not sure where harness should reside in, maybe not in the platform but some helper directory that users can have access to?

[alexreinking]

I'm not sure where harness should reside in, maybe not in the platform but some helper directory that users can have access to?

I'm not convinced users need access to our Gemmini test harness? I thought this was fine in tests.

[yamaguchi1024]

I think we should have a helper function that generates the main function exposed to the user, because otherwise, it will be too demanding for novice users to "just run" Exo. I think we don't want a novice users finding Exo repository and trying to use it, and realize that they need to write the main function to call procedures by themselves and drop it.

[alexreinking]

I think we should have a helper function that generates the main function exposed to the user, because otherwise, it will be too demanding for novice users to "just run" Exo. I think we don't want a novice users finding Exo repository and trying to use it, and realize that they need to write the main function to call procedures by themselves and drop it.

I get your point and I generally agree, but I think this merits a much more careful design. Halide has similar features (RunGenMain) we should look at.

I don't think moving this file is necessary for the purposes of artifact evaluation (i.e. this PR), so let's revert that part (moving the harness into the compiler source) for now and we can discuss how to generate command line tools (and perhaps more! e.g. Python/Matlab/Julia bindings, etc.) later.

[yamaguchi1024]

I didn't know about RunGenMain, that sounds super relevant!
Sounds good, let's discuss about this more carefully. I'll remove the harness.

[alexreinking]

Do we not actually want this here? Seems like the tests ought to skip if chipyard isn't available.

[yamaguchi1024]

It won't compile&run gemmini tests unless RISCV is present, because of this line:
https://github.com/ChezJrk/exo/blob/master/tests/gemmini/harness_gemmini.py#L247

Without RISCV environment, it'll just run the frontend.

[gilbo]

Why does this need to go in platforms? Also, relatedly we want to remove platforms from the source ultimately, not add to it.

…

On Apr 5, 2022, at 10:49 AM, Alex Reinking ***@***.***> wrote:  I'm not sure where harness should reside in, maybe not in the platform but some helper directory that users can have access to? I'm not convinced users need access to our Gemmini test harness? I thought this was fine in tests. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because your review was requested.