pulp-platform / snitch_cluster

An energy-efficient RISC-V floating-point compute cluster.
https://pulp-platform.github.io/snitch_cluster/
Apache License 2.0
51 stars 52 forks source link

Fix axi dependency in Bender #173

Closed Lore0599 closed 3 months ago

Lore0599 commented 3 months ago

The axi vcs-fixes branch has been merged into the master branch some days ago. The Bender file is still trying to point to that branch. I change the revision to the last axi commit.

colluca commented 3 months ago

I agree we should fix this but probably we can wait for a new AXI release, right? As long as Bender.lock points to an existing hash, this should not be an issue. Or are you experiencing errors?

Lore0599 commented 3 months ago

Yes I had issues with the chimera repo. Bender said that it couldn't satisfy the requirement 'vcs-fixes'.

colluca commented 3 months ago

I see, so when using the Snitch cluster as a dependency. I think I understand now, presuming the lock file is ignored when considering the requirements of the Snitch cluster as a dependency. I will go ahead and merge this.

@fischeti do you think having removed the Bender.local file might be an issue? We thought it could simply be replaced by tracking the lock file, which is true for the Snitch cluster standalone, but I'm not sure when used as a dependency (for e.g. the reasons mentioned in this post), since I guess the lock file is not considered, whereas I believe the Bender.local file is.

fischeti commented 3 months ago

@fischeti do you think having removed the Bender.local file might be an issue? We thought it could simply be replaced by tracking the lock file, which is true for the Snitch cluster standalone, but I'm not sure when used as a dependency (for e.g. the reasons mentioned in this post), since I guess the lock file is not considered, whereas I believe the Bender.local file is.

I don't think tracking the Bender.local is the way to go. The documentation of Bender does not recommend it since it should only be used for local development (since it is frankly kind of a hack). Regarding the Bender.lock file, it is true that this only works for locking the dependency for standalone development of the snitch cluster, but this is simply how it works. When you specify dependencies (incl. the snitch cluster), you need to resolve all potential version conflicts again to create a new Bender.lock file, since bender needs to have a unique version for each IP. The issue that @Lore0599 reported here was simply that the version specified in the Bender.yml did not exist anymore since it was merged/squashed in the AXI repo. But I don't think that the Bender.local file is part of the problem, since I am pretty sure that it will be ignored as well during version resolution.