Comments (6)
Thanks for reporting!
It looks like I completely misinterpreted the spec's description of the swwe
and swwel
properties, thinking that they override the sw
property at RDL compile-time. Instead, setting either to True
results in a hardware input signal that changes the field's behavior.
This means that in fact, the entire block of code you referenced should simply be removed from the elaborate stage.
Should be an easy fix, but I just want to make sure I don't miss any other assumptions.
Thanks again for reporting! It helps to have multiple people interpreting the spec :-).
from systemrdl-compiler.
Revised interpretation results in a slightly different behavior of the FieldNode.is_sw_writable
and FieldNode.implements_storage
utility properties.
If swwe
or swwel
are set to true
or a reference, then the field is capable of being sw writable (perhaps not all the time). Also implies that the field has a storage element.
from systemrdl-compiler.
I think there are two things being intertwined here. The purpose of the swwe
and swwel
properties are to be able to explicitly define a write enable signal in the HW description of the field. However, your initial assumption of this property affecting the sw
property wasn't wrong either and it came from 7.6.1.-d of the spec.
My personal opinion here is that the spec is trying to be too smart. In my mind setting sw=r
and swwe=true
are conflicting settings and should be reported as such. The spec decided resolve these conflicts and as it happens swwe
and swwel
takes precedence due to the compile order so what you have in the code is not wrong and is faithful to the spec.
There is one problem though. A read-write field, by definition, has a write enable signal and normally this is high active by default. So the assumptions described in 7.6.1-d should go both ways and if sw=rw
this implicitly could result in swwe=true
. At the moment the compiler returns swwe
as false unless explicitly defined. I guess it is up you to decide whether swwe
should return true
or false
when not explicitly defined because the spec just opened up the issue but left some areas for interpretation.
from systemrdl-compiler.
I mostly agree with your interpretation. I agree that the combination of swwe=true
and sw=r
makes no sense, but I disagree that the assumptions described in 7.6.1-d should go both ways. Reading through the spec a bit more, I noticed this in the intro paragraph to section 9.6 (or section 7.6 in the RDL 1.0 spec):
Some of these properties perform operations that directly effect the value of a field (rclr, woset, and woclr), others allow the surrounding logic to effect software operations (swwe and swwel), and still others allow software operations effecting the surrounding logic (swmod and swacc)
I believe the critical text that is missing is clarifying that the swwe
and swwel
properties expose a signal where the user has the ability to augment the sw
property at runtime. If a boolean true
is assigned, then it implies the hardware provides an input to the register file that can be driven by external logic. Alternatively if a signal or field reference is assigned, then the state of the corresponding signal or field determines software's ability to write to the field. The polarity of this user-accessible signal is determined whether swwe
or swwel
is used.
Based on that, I feel that the compiler should do the following:
- Keep existing code that overrides the
sw
property based onswwe
orswwel
(with the fix you originally suggested):- If the designer set the
swwe
orswwel
properties to anything other thanfalse
, then thesw
property should be upgraded to writable. - Reason to keep this is to prevent downstream tools from having to deal with odd conflicting scenarios like the one you mentioned:
swwe=true
andsw=r
- Add: Emit a warning to the user. This kind of design input should be discouraged since it doesn't make sense.
- If the designer set the
- Do not override the value of
swwe
orswwel
- These properties seem to be exclusively for adding hardware that augments the field's "writability" at runtime. You're correct that internally, a field will have its own implicit "write-enable" signal if
sw=rw
but I don't think that's what these properties are intended to denote. These properties imply an entirely unique "write-enable from software is allowed" signal that masks the internal "write-enable" signal. - Overriding
swwe
orswwel
would be confusing to downstream tools since it would destroy the ability to distinguish whether the designer wanted an explicit input signal to their block or not.
- These properties seem to be exclusively for adding hardware that augments the field's "writability" at runtime. You're correct that internally, a field will have its own implicit "write-enable" signal if
- No change to the
FieldNode.is_sw_writable
andFieldNode.implements_storage
utility properties.- Since the compiler will automatically warn and patch the value of the
sw
property, these will continue to function as expected.
- Since the compiler will automatically warn and patch the value of the
Does the above sound reasonable? If so, I may add some of this to the spec errata page I have been maintaining.
from systemrdl-compiler.
I think we are mostly in agreement here and I think your suggestion sounds reasonable, I just add a few notes to clarify what I meant.
For a normal read-write
and write
field there is a write enable signal which is basically active when the bus has a valid transaction, it is a write transaction and the address bus value matches the register offset. I agree that the purpose of the swwe
and swwel
properties would be to augment this by AND'ing this write enable with either a field value (SW controlled lock mechanism) or a signal value (HW controlled lock mechanism). Having the option to define these additional write enables as high or low active makes a lot of sense.
Now using these purely as boolean makes much less sense to me. I don't see a use case for selectively setting normal write enables to low active. Also, the presence or absence of (high active) normal write enable should be determined by the value of the sw
property as I wrote earlier. I think the only reason for having the boolean as an option in the spec is to allow the dynamic reference assignment later.
So I think the downstream tools would really only need to care whether swwe
or swwel
has a reference (although technically they could cover the case when the normal write enable is set to low active but like I said, that seems odd to me). That's why I argued that setting sw
based on the true/false
status swwe
value seems unnecessary but if it is done it may as well go both ways.
So all in all, I agree with the plan that you described. This will be faithful to the spec and will work well for any design descriptions that actually makes sense in HW.
from systemrdl-compiler.
Re-opening based on some discussions I had with @taichi-ishitani.
From section 9.6.1-d:
swwe
andswwel
have precedence over the software access property in determining its current
access state, e.g., if a field is declared assw=rw
, has a swwe property, and the value is currently
false, the effective software access property issw=r
.
I think we were correct to understand that the swwe
/swwel
infers logic to provide a mechanism for a field to be writable by software or not. Upon re-reading this thread, and conversations with Taichi, I don't think it is appropriate for the swwe
/swwel
properties to override the sw
property at compile-time at all. This is purely a runtime effect.
Based on that, I am revising the implementation as follows:
- Never override the value of the
sw
property, regardless of whatswwe
/swwel
is set to. - If
swwe
orswwel
is assigned a signal or field reference, or a boolean oftrue
, then the field must also be writable. If it is not, validation should emit an error.
Also, I think its worth starting a documentation page that provides more detailed interpretations of what logic certain properties should infer. The SystemRDL spec could have been a little more clear on how swwe
/swwel
is supposed to be interpreted.
from systemrdl-compiler.
Related Issues (20)
- regfile inside addrmap does not compile HOT 1
- Cannot alias registers with we = true HOT 2
- Add ability to inject define macros into preprocessor
- Empty addrmap causes compiler to crash HOT 4
- Software-writable field spanning multiple words should cause error message (accesswidth/regwidth problem) HOT 1
- User defined property default for array type causes exception HOT 2
- [BUG] Parameter internal cleanup after first elaboration HOT 1
- Add support for assigning references in importer API
- Unintended `external` applied to `addrmap` instance HOT 3
- missing parameterization in the component hierarchy HOT 13
- Add method to generate a globally scoped type name to Node API HOT 1
- if struct is not fully defined, compiler crashes, it should point the line with to the error in the SystemRDL file HOT 2
- detecting name collisions during path serialization HOT 1
- Allow Elaboration without addrmap HOT 1
- Set abbreviated Node.type_name for long parameterized types HOT 1
- [FEATURE] property_src_ref needed for UserEnum and UserEnumMember
- Describe how addressing is impacted by ispresent in docs HOT 2
- `define for value replacement does not work HOT 2
- Compatibility between UDP `default` property and `UDPDefinition.default_assignment` HOT 4
- [BUG] [QUESTION] - Importer API support for signals HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from systemrdl-compiler.