Create registers from JSON definition #522
Conversation
sparta/scripts/GenRegisterJSON.py
Outdated
| "initial_value": 0, | ||
| "enabled": True})) | ||
|
|
||
| #Include PC, Athena expects the PC to always be 64-bits |
There was a problem hiding this comment.
Can you delete lines 243-265? They don't need to be checked into Sparta.
There was a problem hiding this comment.
Done (including removing resv_valid)
| } | ||
|
|
||
| private: | ||
| void parse(const std::string& register_defns_json_filename) |
There was a problem hiding this comment.
Rename method to parse_.
| document.Parse(json_str.c_str()); | ||
|
|
||
| for (auto& item : document.GetArray()) { | ||
| if (item.HasMember("enabled") && !item["enabled"].GetBool()) { |
There was a problem hiding this comment.
I think this may break backwards compatibility. @furuame can you confirm? From the functional model's POV, I believe there is a difference between "this register is invalid" and "this register is valid but not enabled for this arch".
There was a problem hiding this comment.
Yes, I can confirm. This should be fine. We handle that scenario in JSON level. The enabled field is used to be overridden by the other arch JSON to turn that register off. The disabled register is never created.
| } | ||
|
|
||
| RegisterBase::group_idx_type group_idx = group_idx_map_[group_num]++; | ||
| cached_strings_.emplace_back(item["group_name"].GetString()); |
There was a problem hiding this comment.
Should the group_name be optional as well since group_num is?
There was a problem hiding this comment.
Not sure I understand - I am adding "group_name" and "group_num" to every register json. It was my understanding that neither would be optional.
| import sys | ||
| import pdb | ||
|
|
||
| from RV64_CSR import CSR64_DEFS |
There was a problem hiding this comment.
These libraries are missing from the PR.
| "size": 8, | ||
| "aliases": alias, | ||
| "fields": fields, | ||
| "initial_value": 0, |
There was a problem hiding this comment.
Does this need to be changed to "x0"?
There was a problem hiding this comment.
In the __CreateRegDict method, I remove the "initial_value" key from the dict if it is zero, else convert to hex.
| "size": 8, | ||
| "aliases": ["csr"+str(k)], | ||
| "fields": v[2], | ||
| "initial_value": v[3], |
There was a problem hiding this comment.
Same concern as above, need to make sure the initial values are hex strings.
No description provided.