search

minerva-cpu / minerva

A 32-bit RISC-V soft processor
Other
301 stars 32 forks source link

Some structural changes: #9

Closed BracketMaster closed 4 years ago

BracketMaster commented 4 years ago
whitequark commented 4 years ago
  • don't think whitequark is still pushing to pypi anymore

What the hell are you talking about? nMigen is released on PyPI, and you should discuss release schedule with upstream if you're not satisfied with it before pushing misinformation to other projects. For everyone's sake, please stop making unfounded conclusions and spreading them without ever talking with the people involved.

whitequark commented 4 years ago

I've emailed you to elaborate on that topic.

BracketMaster commented 4 years ago

Ah. Thanks for the correction. I for some reason thought I that was the case.

Hopefully - the rest of the pull request is useful, the codebase as is does require a more current version of nmigen to work with @jfng hello world example than is available on Pypi.

whitequark commented 4 years ago

Yep. I think Minerva itself should work with nMigen 0.2. There were also some issues related to nmigen-soc--we can just make a new release of the latter if necessary.

BracketMaster commented 4 years ago

Yes, there are some dependencies on harry-ho's fork of nmigen-soc for jfng's minerva SOC example.

It took me a second to figure that out.

whitequark commented 4 years ago

Wait, are there? I'm using commit e72f9501c03e358b43d285f88fda0587ab034ad9 from upstream nmigen-soc and I think the example works for me?

BracketMaster commented 4 years ago

Actually, you're right. It does work with upstream. However, I'm still not convinced it works with nmigen .2

So if I do the following:

python3 -m venv test1
source test1/bin/activate
pip3 install git+https://github.com/lambdaconcept/minerva
pip3 install git+https://github.com/m-labs/nmigen-soc
git clone https://github.com/jfng/minerva-examples
cd minerva-examples/hello
make
python3 core.py

I get:

(test1):hello$python3 core.py 
Traceback (most recent call last):
  File "core.py", line 134, in <module>
    sim.add_clock(1e-6)
test1/lib/python3.7/site-packages/nmigen/back/pysim.py", line 477, in add_clock
    .format(domain))
ValueError: Domain 'sync' is not present in simulation

However, if I do:

python3 -m venv test2
source test2/bin/activate
pip3 install git+https://github.com/lambdaconcept/minerva
pip3 uninstall nmigen
pip3 install git+https://github.com/nmigen/nmigen.git
pip3 install git+https://github.com/m-labs/nmigen-soc
git clone https://github.com/jfng/minerva-examples
cd minerva-examples/hello
make
python3 core.py

I get:

hello, world!
whitequark commented 4 years ago

Why are you using m-labs anything? All of those repositories lag behind upstream.

whitequark commented 4 years ago

Anyway, I'll look into what's necessary to get minerva-examples to work with PyPI packages.

BracketMaster commented 4 years ago

I just now discovered nmigen/nmigen-soc.

Anyway, I'll look into what's necessary to get minerva-examples to work with PyPI packages.

OK

BracketMaster commented 4 years ago

Any thoughts on setup.py test and internalizing FHDLTestCase?

whitequark commented 4 years ago

Any thoughts on setup.py test

I haven't looked deeply into it, but wouldn't absolute imports break things if you have one version of Minerva installed globally, and you're testing your local changes in the source?

and internalizing FHDLTestCase?

Yes, and I've already explained to you why blindly copying things out of nMigen's source tree isn't the way to go. The only thing you need out of that file is the assertFormal method, and the odd hybrid mode should be dropped too.

BracketMaster commented 4 years ago

Any thoughts on setup.py test

I haven't looked deeply into it, but wouldn't absolute imports break things if you have one version of Minerva installed globally, and you're testing your local changes in the source?

That is a valid point. Might make more sense to move test back into Minerva dir and just point setup.py to that.

and internalizing FHDLTestCase?

Yes, and I've already explained to you why blindly copying things out of nMigen's source tree isn't the way to go. The only thing you need out of that file is the assertFormal method, and the odd hybrid mode should be dropped too.

Will edit.

whitequark commented 4 years ago

Will edit.

Once the documentation is ready (I'm going to work on that after cxxsim) there's going to be a topic on formal verification with nMigen, so I don't have to hunt down these changes in random GitHub issues.