Add pulp_cluster target
#37
Conversation
| print_summary(errors); | ||
|
|
||
| if(get_core_id() == 0) | ||
| print_summary(errors); | ||
|
|
There was a problem hiding this comment.
Can you provide some reasoning to why this is necessary?
There was a problem hiding this comment.
It is not necessary. Right now, all the cores print the same exact summary and result, as they all get the number of cycles from the same timer and errors is a shared variable. That's why I thought to simply have one core printing it but I can remove the if(get_core_id()==0) 😄
There was a problem hiding this comment.
@bluewww, any inputs on this? 🚀
There was a problem hiding this comment.
I think this needs to be removed
kernel/cluster.c
Outdated
| @@ -43,16 +42,24 @@ static void cluster_core_init() | |||
| eu_bar_setup(eu_bar_addr(0), (1<<ARCHI_CLUSTER_NB_PE) - 1); | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
There are a few whitespace changes in this PR, some adding unneeded spaces. Can you remove these?
There was a problem hiding this comment.
I hope I removed all the whitespaces!
rules/pulpos/default_rules.mk
Outdated
| ifeq '$(pulp_chip)' 'pulp_cluster' | ||
| run: | ||
| vsim $(vsim-flags) -do "set VSIM_PATH $(VSIM_PATH); source $(VSIM_PATH)/scripts/start.tcl" | ||
| else |
There was a problem hiding this comment.
Is this completely separate target for pulp_cluster required? Is it possible to use the same flow as below? If not, I suggest to keep as much as possible aligned (e.g. using $(VSIM) instead of vsim to start vsim).
There was a problem hiding this comment.
This one is a bit tricky: I didn't check the existing flow since the pulp cluster tb is very much different from pulp's tb. I could dig into the "baseline" scripts and modify the cluster's testbench to become compatible. However, I honestly think that the pulp flow is a bit convoluted and overkill for our needs, aligning new code to a legacy version maybe it's not the best approach, but I see why avoiding splitting into different targets.
There was a problem hiding this comment.
These is the default_rules.mk file which should apply, well, to things that should be default. Custom stuff should go into pulp_cluster.mk. You can just overwrite the run: target in pulp_cluster.mk with whatever else you want it to be.
There was a problem hiding this comment.
Yeah, you are right. This makes more sense!
There was a problem hiding this comment.
As you suggested, I am now overwriting the run target within the pulp_cluster.mk. No need to modify the default_rules.mk at all this way.
rules/pulpos/default_rules.mk
Outdated
| else | ||
| vsim-flags = -c |
There was a problem hiding this comment.
This should already be added in the default flow, no? I see that it is needed in the custom flow for the cluster, but...
There was a problem hiding this comment.
runner_args is currently used only when the target platform is gvsoc. I can work only with runner_args and define it according to the target, what do you think?
luca-valente
force-pushed
the
lv/pulp_cluster
branch
2 times, most recently
from
3041646
to
594cf12
Compare
Core 0 does the initialization as if it was the FC and then all the cores enter the main with the proper stack initialization.
luca-valente
force-pushed
the
lv/pulp_cluster
branch
from
594cf12
to
41b428d
Compare
|
Let me know if other changes are needed! |
* Pass `APP` variable to the startup TCL script * Change the executed startup script based on gui or batch simulation
ARCHI_NO_FCto enable the cluster to boot on its own