Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pattern internals #30

Closed
wants to merge 12 commits into from
Closed

Pattern internals #30

wants to merge 12 commits into from

Conversation

pderouen
Copy link
Member

This is related to implementation of #18 and #19.

Very early on in the process. This PR is to collect input (and contributions if you like) on the implementation details.

pub use super::operation::Operation;

pub struct PinAction {
name: String,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in the end this should be an immutable reference to a pin/group. I would guess the reference will give a smaller memory footprint and allow more flexibility when processing the actions.

@coreyeng coreyeng requested review from coreyeng and ginty December 19, 2019 16:41
Copy link
Member

@coreyeng coreyeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pin actions in #21 already. Since I have the pin implementation there and its dependent on the action, I'd rather use what I already have instead of trying to integrate with this.

Stephen may already have something for register states as well.

@pderouen
Copy link
Member Author

I think this is intended to be something different. This is just the storage element for the internal pattern action object. Is this truly a duplicate?

@pderouen
Copy link
Member Author

pderouen commented Dec 19, 2019

I just found it in your code. I am duplicating. My Operation enum duplicates your PinAction enum. It makes sense for those to live with pins. I will use your enum when it's merged. Though I foresee a need to have several more actions available. I've defined the types of things we typically do with pins in addition to what you have.

My PinAction struct is the storage element to contain the pin reference, data, and operation (which is equivalent to your PinAction).

@pderouen
Copy link
Member Author

@ginty, @coreyeng, kind of a procedural question. Should the pattern internals define the storage structs for each "action" type. Or, should pins (for example) define it's own structure that the pattern actions struct reuses?

@coreyeng
Copy link
Member

coreyeng commented Dec 19, 2019

I'd think it should reuse the existing structures instead of re-implementing them. I think instead of casting from, say, a Pin to a PatternInternal::Pin, you should just ask/open tickets for what you need. Or, add it at the pin/register level (when we check it in. I hope to merge #21 in the next couple days).

To answer your question from before, it looks pretty close: https://github.com/Origen-SDK/o2/blob/pins/rust/origen/src/core/model/pins/pin.rs#L5-L41

It also goes with your immediate question: I think you'd want to reuse the O2 primitive pin and register instead of "casting" it. It'll be both slower and adding a complexity level I don't think we'll need.

@pderouen
Copy link
Member Author

@coreyeng I still don't think we're talking about exactly the same thing. Yes, I do plan to simply grab the enumeration from pin that you reference once it's available. That describes the different things that you can do with a pin. My question is slightly different. When the user does something like this:

    dut.pin("porta").drive(7)

That information gets stored to a struct in the "pattern internals" as:

    // type_of_thing_being_recorded; necessary_information_about_the_thing
    //       Should Pattern Internals Define this Struct between [ ] below?
    //        Or, do we want to define it as part of pins?
    <Pin; [ref_to_porta, "0x7", YourPinActionEnumerationValueHere]>
  
    // other kinds of things get stored
    <Reg; [ref_to_reg, data, Type_Of_Action(which is maybe the same list as for a pin)>
    <Cycle; repeat count>
    etc.

This struct kind of belongs to both pins and pattern. It's kind of a question of do we centralize the definitions for all of the storage structures, or do we let the module doing the storing own the definition? For consistency in construction centralizing makes sense. But, it also makes sense to keep all things pin in the same module.

@pderouen
Copy link
Member Author

What we're talking about doing in #18 is taking a snapshot of a subset of the information here (storing all of it isn't necessary, we just need the new state):

pub struct Pin {
// Since pins will be added from the add_pin function of Pins,
// just reuse that String instance instead of creating a new one.
pub id: String,
pub path: String,
pub data: u8,
/// The pin's current action. If no action is desired, the pin will be HighZ.
pub action: PinActions,
/// The pin's initial action and state. This will be applied during creation and whenever the
/// 'reset' function is called.
pub initial: (PinActions, bool),
///--- Meta Data ---///
/// Any aliases this Pin has.
pub aliases: Vec<String>,
pub role: PinRoles,
pub meta: HashMap<String, MetaAble>,
// Taking the speed over size here: this'll allow for quick lookups and indexing from pins into the pin group, but will
// require a bit of extra storage. Since that storage is only a reference and uint, it should be small and well worth the
// lookup boost.
pub memberships: HashMap<String, i32>,
}

I envision this method

pub fn set_pin_actions(&mut self, ids: &Vec<String>, action: PinActions, data: Option<u32>) -> Result<(), Error> {
if let Some(d) = data {
self.set_pin_data(ids, d)?;
}
for (_i, n) in ids.iter().enumerate() {
let p = self.pin(n).unwrap();
p.action = action;
}
Ok(())
}
creating an instance of the smaller pin struct, populating it with the same info that is stored to the larger pin struct and then passing it off to the pattern action storer (which is just a Rust vec that stores instances of an enum struct of all different types of "actions").

Then, the pattern outputer, or protocol aware outputter, or transaction outputer will iterate over the vector of stored stuff and create whatever target and type is requested.

@ginty
Copy link
Member

ginty commented Dec 20, 2019

Sorry to interject here with a potential bombshell, but I think @info-rchitect had a good point yesterday about patterns either now or in future having to handle some level of conditional logic/branching.

What we are really creating here is a poor man's AST representation of the pattern, but after reflecting on the above, I think we should be going for a proper AST representation and then all ATE drivers can be implemented as AST interpreters.

That also has the advantage or working out how to do that in Rust since we will need it for the program generator anyway, and it would be nice to bring more alignment between our two primary generators. Maybe we will even find some utility in future from being able to build a kind of super AST that contained absolutely everything about a test flow from the highest test program concern right down to the lowest pin level detail.

I can't remember if you were invovled much in the O1 program generator internals @pderouen, but if not I found that the documentation for the ruby gem we used really helped me to understand the concepts of how a formal compiler works - http://whitequark.github.io/ast/AST/Processor/Mixin.html

The bad news is that Rust does not exactly seem to be overflowing with similar tutorials, though this one looks pretty good - https://fitzgeraldnick.com/2018/11/15/program-synthesis-is-possible-in-rust.html
You can ignore the parsing front end stuff, but basically it work similar to what we are talking about here, but instead of building up this list of custom stucts it would add nodes into the AST - which are similar to structs you are already planning. And I think such nodes should be owned by the patgen code rather than in pin, reg, etc. Agreed that their job is to distill down exactly what you need to know about an objects state at that point in time and nothing else.

I think the hard part here is going to be working out what Rust incantations are required to add the AST nodes. The interpretation part in that tut looks pretty clear and we should be able to adapt that easily enough to have it spit out lines to a pattern file.

I note that that tut uses this arena crate which looks interesting - https://crates.io/crates/id-arena

I think we should get how that works under our belt too, since that seems to offer a solution of how we make references to pins/regs/etc. more directly rather than via a String-based lookup like we currently doing.

Thoughts?

Sorry for potentially throwing a spanner in the works, but better to re-align now than later.

@ginty
Copy link
Member

ginty commented Dec 20, 2019

The Visitor (interpreter) part of this looks pretty good - https://thume.ca/2019/04/18/writing-a-compiler-in-rust/

@ginty
Copy link
Member

ginty commented Dec 20, 2019

Another good one - https://dev.to/deciduously/rust-your-own-lisp-50an

@pderouen
Copy link
Member Author

@ginty this is the right time for that kind of feedback (I have not spent a ton of time on it). I was not very involved with the O1 program generator. I only recently started tinkering with it. I found the internals of it to be difficult to comprehend and difficult to follow the logic. I'm happy to take a look at an AST implementation of the internals. I'll get back with you in a few days to weeks.

@pderouen
Copy link
Member Author

@ginty are you envisioning a pattern generation process similar to O1's program generation, where (for example) the JTAG driver is implemented through recursive method calls a node tree? If we want to be able to take the internal pattern representation and target multiple output formats (ascii pattern text, protocol aware pattern text, driver level text, simulation), it seems like the methods for processing the nodes down to output have to be either:
A1) de-coupled from the nodes (this makes the most sense to me)
A2) or, the nodes need to be configurable and include all logic for all possible outputs (seems needlessly complex to me)

To avoid either of those options I think the only alternative (calling this option B1) is to preselect the targeted output so that the appropriately methodized (I just made up that word) node types can be selected. To my mind this option was taken in O1.

Am I aiming for A1, A2, B1 or something else entirely? My suggestion would be A1 (the data is represented in a node tree de-coupled from the processing).

My thoughts on implementing conditional logic handling are along these lines (just made up syntax to illustrate the structure):

    tester.if_true(dut.pin("a_pin").read(0))
        // pin, register, etc. actions created in this block
       // will belong to a new instance of a pattern action collector
    tester.else
        // same as above block, these go to a new action collector
    tester.end_if

Sim or Link will simply perform the read and branch during execution. An ASCII pattern generator would generate both branches and insert the appropriate microcodes to perform the compare and branch at pattern run time.

@info-rchitect feel free to chime in.

@ginty
Copy link
Member

ginty commented Dec 21, 2019

Hi @pderouen,

I think by default a call to jtag.write_dr(0x1234, size=16) would add something like these nodes to the AST (written in s-expression format, each 's' is a node):

s("write_dr",
  s("data", 0x1234),
  s("size", 16),
  s("pin_action",
    s("pin_id", "tms"),
    s("operation", "drive"),
    s("data", 1)),
  s("pin_action",
    s("pin_id", "tdi"),
    s("operation", "drive"),
    s("data", 1)),    
  s("cycle", 1),
  s("pin_action",
    s("pin_id", "tdi"),
    s("operation", "drive"),
    s("data", 0),    
  s("cycle", 1),
  ...

AST interpreters work by handling the type of nodes they understand and then for the rest they continue to process down through their children, but otherwise ignore them.
So, for example a vector-level ATE driver might ignore the write_dr node, but it would process down into its children where it would find pin and cycle nodes that it does know how to handle.

A JTAG protocol-aware interpreter would consume the data and size attributes of the write_dr node, but just ignore the pin and cycle nodes.

From some of the links above, it looks like a a Rust interpreter would be implemented by a putting every node through a match (pseudo code):

// vector-level interpreter
match node {
  "pin_action" => update_current_vector(node),
  "cycle" => render_current_vector,
  // For all other node types process their child nodes, but otherwise ignore
  _ => process_children(node),
}

// An alternative JTAG PA interpreter
match node {
  "write_dr" => render_write_dr(node),
  "read_dr" => render_read_dr(node),
  "write_ir" => render_write_ir(node),
  "read_ir" => render_read_ir(node),
  // For all other node types process their child nodes, but otherwise ignore
  _ => process_children(node),,  
}

I think what you're getting at above is that it's wasteful to put the pin-level nodes in the AST if you have no intention of interpreting them later.
In that case the jtag driver could just have a switch to make it skip the pin-level operations to save some time.

Hopefully that helps?

@ginty
Copy link
Member

ginty commented Dec 21, 2019

where (for example) the JTAG driver is implemented through recursive method calls a node tree?

No, the JTAG driver would be much the same as the O1 JTAG driver.
The only difference is that it would also be responsible for placing the JTAG PA-level nodes into the AST.
But otherwise it would just call pin("tdi").drive(0), tester.cycle and so on as it does in O1 and then it is the pin and tester methods which are responsible for adding their info to the AST.

So the JTAG driver just does some "push and forget" operations into the AST, but it doesn't do any walking through it.
All the walking of the AST is done by the tester drivers to process it into their specific output format.

@pderouen
Copy link
Member Author

I think by default a call to jtag.write_dr(0x1234, size=16) would add something like these nodes to the AST (written in s-expression format, each 's' is a node):
s("write_dr",
s("data", 0x1234),
s("size", 16),
s("pin_action",
s("pin_id", "tms"),
s("operation", "drive"),
s("data", 1)),
s("pin_action",
s("pin_id", "tdi"),
s("operation", "drive"),
s("data", 1)),
s("cycle", 1),
s("pin_action",
s("pin_id", "tdi"),
s("operation", "drive"),
s("data", 0),
s("cycle", 1),
...

This is what I was calling option A1. The data tree is just structured data.

I think what you're getting at above is that it's wasteful to put the pin-level nodes in the AST if you have no intention of interpreting them later.

No, it seems likely that I just misunderstand the underlying implementation in O1. I thought each node was intended to have the interpretation methods embeded inside the node. This is what seemed wasteful to me, because a pin node for example would have to know how to process itself based on the desired output format: uflex atp, 93k, sim, link, etc.

@coreyeng
Copy link
Member

@ginty, @pderouen, sorry, I've not been caught up on everything here.

Just to interject one quick thing as I read and digest everything, I see a lot of references being passed around in the examples, which require lifetimes. Pyo3 has pretty shoddy lifetime support, so unless we're okay with making those static (for some things, that should be fine), we may need to be careful with the exact architecture.

A mod which contains all of these at the DUT level may be a more-verbose-but-lifetime-independent way of handling this. Or requiring a very-defined naming structure. Unless we can figure out lifetimes in pyo3 (which I wasn't able to make much progress on outside of just static).

@ginty
Copy link
Member

ginty commented Dec 27, 2019

@coreyeng, I expected that the AST would be built up in a static variable similar to the DUT. Probably PATTERN would be a vector similar to #40, and it would get cleared at the start of each pattern.

@pderouen
Copy link
Member Author

Just wanted to give a quick update. I've done nothing at all for the past week or so. I'm looking at id_arena to implement the AST. It seems simple enough. I'm also reading up on some Rust coding concepts. I think I will have another swag at the internal pattern representation soon. I will close this PR in a few days and then open a new one at some point after that.

@pderouen
Copy link
Member Author

closing this PR, will reopen soon

@pderouen pderouen closed this Dec 31, 2019
@coreyeng coreyeng mentioned this pull request Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants