Tester prototype

[coreyeng]

Hello,

I've got a prototyped tester interface that I think is in good enough shape to start looking at. A few quick notes first in case anyone wants to pull it down:

• I've got an AST and some nodes stubbed out (as indicated by actually calling it StubAST!) but these are just a proof of concept and are by no means complete. Eventually, all this stubbed stuff should be replaced with Pattern internals #30
• I've really only tested for feature coverage at this point. There's many holes that still need to be filled, some of which will generate not-so-friendly panics. Filling those holes isn't worth the effort if we reject the structure though, so I'm leaving them open for the time being. I'll go back and plug them up if we like the structure here.
• I've got specs for the major features though, so see the general usage here.

I went in a different direction than what we have for O1 (and as a result, xfailed this compiler test for the time being). All of this is open for discussion and I just kinda went with a gut feeling to start. Firstly, origen.tester is now 'always' available, with always in quotes because certain things may raise errors if, for example, a timeset hasn't been set. But, point is, it won't be None.

The main reason for this is that inheritance is a bit trickier in Rust, then throwing frontend support on top of that lends itself better to an adaptor pattern than just straight inheritance (via mixins) that we usually work with in Ruby. Instead of having a base which the O1 testers would inherit from, we have a topmost interface, which O2 generators will plug into. Now, origen.tester provides a constant API to build up an AST and set and register generators. It’s the generators responsibility now to produce some output, given the AST.

The pattern source will more or less be a straight translation from O1. Given just the basic API, a simple pattern would still look like this.

Secondly, I've always had a heck of time trying to convince new users, especially those not very familiar with programming in general or with non-native English speakers, that setting Origen's environment is separate from the OS's environment itself (the platform they're running on, environment variables, etc.). Since we're going from the ground up, I instead dual-purposed the target nomenclature, which I'm 50/50 on myself. When it comes time to actually generate the output, the frontend will iterate through all the targeted generators and produce an output for each one. I don't currently have anything using file IO, but I threw together a dummy V93K one that prints would-be comments and vectors to the console:

I've got some test generators still floating around, but as we flush out the real ones we can remove the test ones.

So, that's the big thing. We now have a tester with some of the elementary O1 API working and are able to use the frontend to build up a dummy AST, and use a Rust-powered backend to generate some output.

This use case is what I think will be the most prominent. Ideally, we'd keep all the generators in the backend and use the AST directly to produce an output. However, I think that'd be a little too naïve to only support that, so I added some extra features to make sure the structure is flexible for what I'm guessing would be feature requests that'd come up pretty quickly.

Frontend Generators

If needed, you can write a generator in the frontend, register it, and then use it like one in the backend. This'll allow generators to be prototyped or written completely without needing to know Rust and requiring new backend builds. This'll have some performance impact compared to generators that operate entirely in the backend, but that's to be expected. Anyway, here is an example of creating a frontend generator, and here's an example of registering and using it. Shown below as well:

As we move forward, these will need more context, like the filename to write, which would also come from the backend. Things like that can be easily added though with the given structure. We'd just be passing additional arguments, so it won't be structural problem.

Interceptors/Callbacks

An aspect that I also wanted to make sure we had was callbacks/interceptors from O1. OrigenSim just overrode the cycle and cc methods in O1, but that's easily doable in Rust. Instead, I added the callback/interceptor structure and used that. A test generator with some interceptors is available here, and would have this output when generated:

Interceptors can be used similarly in the frontend as well:

One point of discussion is whether or not we want the interceptors to be able to mutate the AST. Currently, they can, both from the frontend and the backend. I did this 99% to make sure it was possible and that both the borrower checker and PyO3 would even allow such this to occur, and 1% because I think users should be able to. I actually think we shouldn't allow mutable AST access and should instead require them to throw in meta objects (covered next for the frontend), but I don't have "backend meta" working yet. Only on the frontend.

For a practical example of the interceptors which doesn't mutate the AST, there's the dummy OrigenSim generator which would work in much the way it does now… hijack the calls to cc or cycle, do its thing, but ultimately leave the AST untouched.

Frontend Meta

I added meta support to the AST nodes, like I had for pins. I think this gives pretty much unlimited flexibility in that someone should be able to write a generator to do anything, no Rust experience required or even waiting for us to add additional features. Here's a generator which disregards the AST for the most part and instead uses only the node type and the meta objects attached to the node to generate an output. This isn't an overly practical approach, but its possible, which is all I was going for here. The output of this will look like:

Targeted Generator Ordering

One thing to observe, and I did this on purpose, is that the generators will be run in the order in which they were targeted, allowing for utility generators to take effect first.

For example, targeting the PyTestGeneratorWithInterceptor then the DummyGeneratorWithInterceptors produce this output, while targeting them in reverse order produces this output. Notice the reversed order of Intercepted By PyTestGeneratorWithInterceptor and Comment intercepted by DummyGeneratorWithInterceptors!.

Again, whether we allow the AST to be mutated like this in the long term is a discussion point, but this order dependency would apply to meta regardless.

Other Notes

Just some additional things in this PR:

• As per pyo3 0.9 alpha, this note seems to have been removed
  
  I've been using this in 0.8.5 and I've not noticed it causing any problems. I'm assuming this means the subclassing is fine in 9+, which I'm pretty sure they'll hit a non-alpha 0.9 before we're ready to release O2.
• I don't think anyone has used the 'list/dict' like APIs yet, but if so and you had problems with it hogging the dut lock, that's been fixed.
• Also fixed the derive to print the actual field name when a lookup fails instead of the literal config.field that was seen before.
• Regarding errors though… I've noticed some undeterministic python crashes (note: not Rust panics) occurring during the generation regressions. I think this is due to pytest both capturing the output to compare it whilst also trying to dump it to the console using the -s switch… but I'm not 100% sure. I've only noticed the crashes when running regressions and only during one of the generators. It seems too that when CPU usage is high, like when our IT decides to push new virus definitions every other hour or so and Symantec hijacks 50% of my CPU bandwidth for whatever TF its doing () its more likely to happen. I can't get anything consistent though, and I've never seen it happen when I don't use the -s switch, so I'm assuming that's the cause (and, if so, its probably a windows-only thing). I also have never seen just the general tester usage fail, so I don't think its tied to the first bullet at all (the stack trace doesn’t show any pyo3 stuff also). But, if anyone notices it popping up, let me know, because I really want to know what's causing it if its not that.

[pderouen]

Nice! I like the direction you've taken. Thanks for all of the effort you're putting into this. Sorry about the long wait on #30.

[ginty]

Hi @coreyeng, sorry it took a while to look at this properly, awesome PR as always!

Some random comments as a I read through it:

No issue with origen.tester always being present.

Interesting point re environment, I think we should loose it and enhance the target system to effectively allow multiple targets to be selected at once. I'll open a separate ticket for us to discuss the direction of that.

The arch with a common frontend API with different backend generator targets is perfect.

Awesome that you have allowed both Rust and Python-based generators.

I don't really like the idea of AST mutation beyond appending new nodes, but on the other hand being able to modify the recent past was sometimes useful when creating drivers in O1.
So, good you allow it, but this ability should be well abstracted from user space, i.e. I would never give user space a direct mutable handle on the AST.

Great to see the OrigenSim target already stubbed out!

On tester-specific stuff, the ability to add arbitrary meta data is powerful and a great feature, however for some ATE-specific features it would be good to see proper AST support too.
For example, the UltraFLEX allows tester instrumentation control from within pattern code and I think I would expect to see a dedicated node type for something like that.
So does the arch here allow for ATE-specific nodes to be defined and then backend targets which support them could use them like normal nodes while other targets would simply ignore them?
Even though such nodes/APIs would be defined in ultraflex.rs (for example), they would be universally available on origen.tester, probably namespaced would be ideal: origen.tester.ultraflex.some_instrument_operation().
That would make it clear to the user that they were using an ATE-specific API, yet it would be better than O1 in that they would not have to guard that with if tester.is_ultraflex() because non-supporting targets (e.g. OrigenSim) would just ignore it naturally.
So the application always has the complete tester API available, and wether they have selected target/v93k.py or target/uflex.py simply determines who is going to process the AST to an output.

I find the last comments about generator order a little confusing.
If we are talking about callback/interceptor calling order being undefined, then I think that's OK, O1 was like that and if the application really cares about ordering then it should not be hard for it deal with that on the application side.
With regards to having multiple backend/generator targets selected at once (which is something I think we should support), then they should not affect each other at all.
Backend generation should definitely not be done by AST mutation, any generator that wants to change the AST must create a new one.
That's normal for compilers to go through various AST transforms, maybe we would have an initial pass to clean up some things and validate, then another one to vectorize (more on this below), then another one to do the vector compression, each of these steps would produce a new (immutable) AST instance which is more refined than the one before.
The final AST should be structurally very similar to the final output you want to produce and so the final pass throuh the AST to write to a file should be trivial.

If we have multiple targets then once one is done then the next one would start working on the master AST which should have been unmodified by the previous generator, then going on to do its own transformations.
Eventually we should evolve to having multiple backend threads each working on their own ASTs in parallel. There should be no borrow issues from them all reading the master AST in parallel since it is immutable by this stage - i.e. mutation should only be allowed during the AST creation stage, once it is considered done and we move to backend generation, then it is immutable.

One comment on the API/AST is that I see you have a vector node, probably because we had it in O1. We should not have the concept of a vector in the top-level AST, only pin changes, cycles and timing information.
For a vector-based tester, it would be one of the AST transforms to turn that into vectors.

Finally I have never seen PyTest crash when running with -s, which I do quite a lot. I am running on Linux though (well, WSL2 on Win10).

[coreyeng]

Hi @ginty, thanks for the feedback!

Interesting point re environment, I think we should loose it and enhance the target system to effectively allow multiple targets to be selected at once. I'll open a separate ticket for us to discuss the direction of that.

It was really only an issue with users pretty new to programming overall or, in some smaller cases, an english-second-language thing of not fully grasping how the term environment applies to the tester. What I have here was just to get away from environment and I'm open to changing that to something else. Just not sure what that 'something else' is.

I think most of the change requests are AST related, but just to point out again real quick that for all the AST stuff, this was only meant to be a stub so I could show some real examples of the tester and generators working. I fully expected this to be replaced when we knew more AST details, and actually thought this would be ripped out as a whole. Though, if the direction here is salvageable, I can definitely develop the AST some, it just wasn't my intention and I put very little effort it, outside of what I wanted to show with the generators.

I don't really like the idea of AST mutation beyond appending new nodes, but on the other hand being able to modify the recent past was sometimes useful when creating drivers in O1.

Those were my thoughts as well, though I wasn't really sure what a user API might look like and what would be useful. Going with the above point though, I decided just to give them the entire AST to 'prove the point' rather than as a real feature. I think we can open a ticket to discuss what a manipulations a user should be able to do to the AST and I can build those in. The fact that I can give them the AST at all should mean that we can easily provide them functions instead and not run into borrower or conversion (between Python & Rust) issues.

So does the arch here allow for ATE-specific nodes to be defined and then backend targets which support them could use them like normal nodes while other targets would simply ignore them?

Not currently in the way described, though that could be added. Currently, we could add whatever ATE-specifics we wanted and the generator could just choose to ignore it if its unsure what to do with the node type (for example, the OrigenSim stub ignores everything!), going along with your note of:

So the application always has the complete tester API available, and wether they have selected target/v93k.py or target/uflex.py simply determines who is going to process the AST to an output.

I was avoiding ATE specifics in the AST because I think it might over-complicate the AST. It seemed more straightforward for the ATE to instead shift in a generalized meta node, then it'd be responsible for checking for that node and using it appropriately during generation and it'd be completely transparent to any generators not looking for it.

If we are talking about callback/interceptor calling order being undefined, then I think that's OK, O1 was like that and if the application really cares about ordering then it should not be hard for it deal with that on the application side.

Actually, there was no way in O1 to deal with this easily. It was actually due to the interceptors and callbacks iterating through the plugins which 1) were ordered based on the Gemfile (expected, but not changeable by end users) and 2) the plugins list being dynamically generated each time it was called, meaning that monkey-patching our way around it wasn't easy either. I actually added the order dependency here for that reason, since monkey-patching the order dependency wouldn't be an option in a compiled backend.

As for the context, my thought was if we wanted some sort of 'pre-processing' of the AST from the user's perspective, or other 'utility-like' generators, those could be loaded first, do their thing, then the actual generators would start up. I'm not sure how useful this would be (sounds like 'not very') and it was easier to add it now and remove later than vice-versa. Again, not being sure of how the AST will look in the end, I just wanted to be sure we could do these things, if the need arose.

Regarding all the other AST/vector node comments, I agree, and I actually left off pin states for that reason and just focused on something simple, but would do something (even if just comment generation). I know you want to go with a more protocol-aware AST and only jump into actually driving pins if the generator necessitates it. Again, not sure how that would look and didn't put much thought into just a stubbed AST for prototyping but can definitely pick this up.

Finally I have never seen PyTest crash when running with -s, which I do quite a lot. I am running on Linux though (well, WSL2 on Win10).

-s has been fine for me until I started using the output in the test cases, as in here. Every time I get an exception, its during one of these tests, and I never noticed it either until I began hijacking the captured output. Here's the bottom of the exception though:

It originates in ansitowin32 fromcolorama (it also screws up the coloring for the entire shell after this). Seems windows specific to me based on the file name and (more importantly, IMO) unrelated to anything we're doing with PyO3. I'm curious to know if this is a problem with only Windows though. It could be a configuration problem of colorama on Windows, but its undeterministic so hard to track down the exact cause. Need more data points...

Anyway, what would be the next steps for this PR you'd like to see? Do you want to reuse the AST stuff in place here as kickstarter? Instead of a wholesale replacement, like I was expecting?

[pderouen]

If there are more pattern AST node types that you know could be used right away, I'd be happy to add those (can do fairly easily) to #48 while I'm working on making the connection between Python and Rust.

[coreyeng]

@pderouen I'm not sure of what's all needed yet. I think in general adding new nodes as needed shouldn't be too bad. They'll probably a few times which we'll need to add new stuff.

[ginty]

Hi @coreyeng, thanks for the quick and full reply.

I've open up this ticket to discuss the environment thing, so we can take any further discussion on that over there - #88

I think we can open a ticket to discuss what a manipulations a user should be able to do to the AST and I can build those in.

OK, sounds good. In my view, I think its really just a negative offset argument that's required to a few key APIs, something like tester.capture(offset=-2) to insert a capture two cycles ago.
I don't remember ever using O1's abilities much beyond that.

I do think we should have tester-specific AST nodes, just stuffing things in meta is just an informal way of doing the same thing - i.e. if the backend can generate something from it then it should be a formal API.
I would tend to see meta-data more for documentation data and not for something that actively influences the generated output.
I don't really see much concern about complicating the AST, because only the tester that can handle such a node will ever look at it.

Probably we need @pderouen's branch to be finished before we decide how we want to lay out the node definitions, so we don't need to decide it right now.

As for the context, my thought was if we wanted some sort of 'pre-processing' of the AST from the user's perspective, or other 'utility-like' generators, those could be loaded first, do their thing, then the actual generators would start up.

Yeah, I could definitely see that being useful and good you have added it.

Anyway, what would be the next steps for this PR you'd like to see? Do you want to reuse the AST stuff in place here as kickstarter? Instead of a wholesale replacement, like I was expecting?

As a reviewer it is pretty hard to envisage here exactly what is final intent and what is just placeholder/throwaway.
It sounds like a lot of this is just prototype which is expected to be replaced and I'm honestly not a fan of merging in a lot of stuff which is expected to be ripped out later.

I think we should discuss with @pderouen at this week's core team what the status and expectations for his branch are.
I think we need to get that merged first to provide a platform for node definitions, then this should be aligned with that and go next.

What do you think?

[coreyeng]

Hi @ginty, thanks for the additional feedback!

OK, sounds good. In my view, I think its really just a negative offset argument that's required to a few key APIs, something like tester.capture(offset=-2) to insert a capture two cycles ago.
I don't remember ever using O1's abilities much beyond that.

Not knowing exactly what's needed is why I'm always inclined to just provide everything. It may be I need a mentality switch since a compiled backend allows us immutably in anything not exposed, whereas all Ruby (or even all Python) allows full mutability via monkeypatching and metaprogramming, provided you know where to look, which is something I do quite a bit when I'm in a time crunch or someone is twiddling their thumbs waiting on an update from me. That said, I can take the current 'full-mutability' and start an API with the 'tester.capture' as a use case. Once that's in place, it should be pretty easy to additional things as it comes up and we can decide from there where to draw the line. I honestly don't know the answer, IMO there's arguments for both sides.

I do think we should have tester-specific AST nodes, just stuffing things in meta is just an informal way of doing the same thing - i.e. if the backend can generate something from it then it should be a formal API.
I would tend to see meta-data more for documentation data and not for something that actively influences the generated output.

That's okay by me. I didn't go much into tester-specifics, but I also like meta as a safeguard or last resort, in this case. I can see a scenario in which an unexpected use case comes up and instead of being dead in the water waiting for us, meta nodes and a Python-side utility generator could provide a workaround and a means to generate a working base case to build into the backend. Again, going back to Ruby where I can easily metaprogram my way into patches allows for quick turnaround while a real patch is worked on. Also again, may need to change my mindset and adopt more software-engineering principles over the hacker mentality with O2.

As a reviewer it is pretty hard to envisage here exactly what is final intent and what is just placeholder/throwaway.
It sounds like a lot of this is just prototype which is expected to be replaced and I'm honestly not a fan of merging in a lot of stuff which is expected to be ripped out later.

I thought the final intent was clear, so my mistake if it wasn't, but with this PR I wanted to show the Tester -> AST -> Ouput chain where the AST is TBD. Though the output stage is raw (the V93K obviously doesn't generate anything meaningful), the path is there and the V93K alone could be further developed to yield a real pattern (also why I added the pin header stuff). I'm confident that the generators will see or more less what they do now; that is, a node in which they'll run a match on and either ignore it or generate something with it. The nodes themselves may change with #30, but the process is there. Also having the StubAST specifically targeted as, well, a Stub I think allows me to develop the aspects around it without having to worry about how the AST does what it does (put another way, I took plenty of shortcuts in the AST specifically so I could work on the generators and tester).

Most of this wasn't meant to be ripped out, just a rudimentary base for us to build off of. For example, with this, I could start working on the entire source -> output process, where the StubAST in there now is transparent, allowing me to further develop while #30 is in development (or maybe I switch to trying to help out with that?).

I'm honestly not a fan of merging in a lot of stuff which is expected to be ripped out later.

I would normally agree with this, but I also think this project is early enough to where churn will be expected. Since its also a small dev team right now I'm not too concerned with conflicts, but if we let this stew for a while it'l become more and more of a "tester prototype + ..." (as it already has) as I'm building other stuff based on this.

The TL;DR; I'm okay with all the above feedback and I really just wanted to PR the tester/generator portions so I can start building the full source -> output path, with a dummy AST for the time being.

[ginty]

but I also like meta as a safeguard or last resort, in this case. I can see a scenario in which an unexpected use case comes up and instead of being dead in the water waiting for us, meta nodes and a Python-side utility generator could provide a workaround and a means to generate a working base case to build into the backend.

That sounds good, just saying that Origen should not do it that way for official APIs. Metadata should be for, well, metadata and maybe doubling as a get-out-of-jail system, but real data should be backed by real data structures.

Also again, may need to change my mindset and adopt more software-engineering principles over the hacker mentality with O2.

Ha, yes I think you should if that's the case!
I'm not really on board with the rush things in approach here, Rust is definitely not as easy to undo as Ruby was (or Python for that matter), and getting the right data structures and arch in place is definitely key to making things work well down the line.

So I am still a bit confused as to what you want to do here, as when I give feedback on the AST not being right you indicate that it is just a throwaway placeholder...

Instead of a wholesale replacement, like I was expecting?

And then when I say, OK then, let's wait until we have the platform in place and do it properly you then say...

Most of this wasn't meant to be ripped out,

I do think it is good that we have two voices arguing for a different approach to this and probably the right way is somewhere down the middle.
I also don't want to block you as your energy for this right now is clearly amazing at driving this forward!

For me though, I still think we should align with @pderouen this week and then decide where we go from here. I don't think he is really that far away from having the AST platform being available.

Then we decide the next level of foundation like where the common AST nodes and APIs are defined and where the ATE-specific nodes and APIs are defined. Add in a few examples and then hook that up to your generation infrastructure.

[coreyeng]

Hi @ginty , thanks for the additional feedback!

So I am still a bit confused as to what you want to do here, as when I give feedback on the AST not being right you indicate that it is just a throwaway placeholder...

The only thing I foresee as being ripped out and reworked completely is the actual StubAST structure. The available nodes here will need updating, but the structure and usage by generators should be similar. Or at least, that's what I'm proposing.

On the PyAPI side, everything here and here would need to be updated with whatever the true nodes are, but I foresee the structure being the same.

The pyclass was providing a fully-mutable representation and from previous discussions we'll move away for that and provide an API. I can begin working on that now though.

But, everything save for the actual StubAST structure should be legitimate. I'm fine with the feedback on the AST, but the AST wasn't the focus. I just needed something for the tester to put stuff in and for the generators to read stuff out of. Granted, that's a pretty big portion, but its only one of three links, with the tester and generators being real proposals. And it's sounding like the tester API, the usage of generators in place of an actual tester like in O1, and the overall way in which the tester adds stuff some AST and how the generators should use it is okay (or maybe we've been too focused on the AST!) and those were the real focuses of this PR.

Hope that helps clear it up some!

I'm not really on board with the rush things in approach here,

That's fine, and normally I'd agree, but its still early stages and I don't think the stubbed stuff is ingrained. I purposefully tried to make that easily replaceable. My concern is that in order to continue to build up the generation portion I need the contents here and I wanted to have the tester API, generators, etc. etc. peer-reviewed, while glossing over the AST.

This'll become and larger and larger PR (as it kinda has with the pin updates) if I keep going. If you're okay with this being a tester prototype + some other stuff PR, then I'm okay with leaving this open. I think the project is still small enough to where keeping this in sync with master shouldn't be a hassle.

[coreyeng]

Hi @ginty,

I think I've got this integrated with the proper AST from #92. I used the process method to go through the AST in generators and I think it worked out pretty well.

I changed the specs slightly to account for immutable ASTs, so the interceptors that were changing the nodes are no longer supported. The interceptors are still there though. I currently have them keying off of tester methods, but I wonder if they should be going off of the processors as well?

I also followed suite in using traits to implement the generators, replacing the match statements I had before. It should make adding a new generator pretty easy and we could introduce some derives in the future to further simplify.

On a different note, what would you think of renaming what I'm currently calling 'generators' to 'renderers' instead?

[ginty]

Hi @coreyeng, this looks good and no issue with introducing the term renderer for the thing that is responsible for creating the pattern output.

See external comment needing clarification.
Thanks!

[coreyeng]

Thanks! I'll update generator to render. That'll take a bit to do, but will still be easier now while its smaller.

I updated the dut, tester, and services methods to fail if they can't grab the lock as well. Like we discussed offline, this would be a development bug but the end users will only see it spin indefinitely, which I think is the worst thing both in their own debug and in being able to give us feedback ("I do this and it hangs indefinitely" - pretty hard to narrow down the issue there).

The only place I saw a problem with this was in the bit collection tests since they are run in multiple threads. I added a dut_or_wait function will which allow for waiting for the mutex to be released if the caller knows that a failed lock attempt won't result in a deadlock. I added respective methods for the tester and services as well.

I changed the dut, etc. methods so this a global change. Do you see anywhere that this might be an issue? Other than the cargo test scripts, everything seems to work.

[ginty]

So, now that we think about it more deeply, I'm not sure that erroring on a lock is the right thing.
Particularly for the backend processors, we should be planning to run them in parallel - either to generate different formats concurrently, or to do things like split large ASTs up into chunks for parallel processing.
In that case, a blocked mutex would be doing its job by holding off access until the thread which currently holds the lock is finished with it.
I think allowing each processor handler to generate its own lock is also good for that since it will result in short lived locks rather than hogging it for an entire processor execution.
If you can come up with a way to differentiate between a deadlock and transient lock then great, but I suspect it is not so easy.
Allowing processors to generate their own locks really doesn't bother me - the only potential I can see for deadlocking would be if a handler method locked the DUT, then called process children directly and then a child handler also tried to lock the DUT. That would only be within the scope of the current handler and so should be easy enough to identify and prevent.
If a handler function is calling out to the wider framework then there shouldn't be any issue since we haven't allowed anything else in rust/origen to generate its own lock and so the handler method would have to pass in the DUT when required anyway.

[coreyeng]

Okay. I wasn't sure where we would try to parallelism, but if we do the AST transmutations in parallel or a divide-and-conquer certain processors, this will definitely get in the way.

I think telling the difference between a transient lock and deadlock might actually be impossible. The best we could do is use the dut_or_wait functions to indicate that waiting is fine, but there's still no way to tell whether a true deadlock occurred. At least not easily.

So, do you think we just go back to dut() (and the others) not generating panics, or do we keep those panicking but use dut_or_wait() in the backend processors? Do we intend to have anything in the frontend run in parallel? I like the idea of trying to do something since I think as the project grows and as we add more static mutexes (like services, the tester, and whatever comes next) it could get more prone to happening and harder to track down. But doing this at the expensive of forcing sequential operation isn't good either.

[ginty]

Hi @coreyeng, I think we should pretty much go back to dut().
I think it is just the nature of the beast that we will have to prevent the possibility of deadlocks somewhat manually.
I think we just need to maintain a clear set of rules about where you are allowed to lock the DUT, so currently locking the DUT is allowed by:

• Top-level functions that are a direct entry point from Python or the CLI
• AST processor handler methods

Areas to be careful:

• Top-level functions that invoke processors will have to release the DUT lock before invoking the processor

[coreyeng]

Okay. We'll just have to watch during PRs that those are followed. The pyapi locking something then calling an underlying method is what I'm afraid of. In general you'd have to know both sides to know where the lock is occurring.

In any case, I reverted the functions and bit collection tests. Should be the same as before now.

[coreyeng]

Hi,

There was bug in this PR regarding the targets: the default (no matter what it is) won't be found. I fixed this and while I was at it, I went ahead and just piled on most of what was asked for in #88.

This updates the target command to be a subcommand. The help message displays as:

The standard origen t does what it did before: displays the currently activated targets. I started adding options to this but it got away from me pretty quickly with needing to watch for --add with --remove, then something like origen t target_1 --add target_2 --remove target_3 and the like and process that correctly. Clap has options to handle all of that actually, but it was also easy, and I think cleaner, to just turn it into a subcommand.

I hope the help dialog is self-explanatory. One thing not mentioned there though is setting targets in other commands. I updated all the existing commands that accepted a target to instead accept a list of comma-separated targets:

For example, origen i --target eagle,sim,v93k_st7 will start an interactive session with those targets activated, run in the order in which they appear.

The target key in the application.toml is now an array, so origen t will display multiple targets. I updated this to store the full paths. In retrospect, I'm not sure that was necessary, but it didn't seem harmful either. For example, origen t will spit out:

A bit more verbose, but also takes care of any resolution, though any failures to resolve should've gotten caught before this.

I added each command calling view after running to show any updates. For example:

In order to do this while reusing the config, I had to make the APPLICATION_STATUS mutable, which could only be done by wrapping it in a mutex. This allows the configs to be refreshed after changing.

I also tossed in some macros to print errors saying 'something went wrong in the backend'. I think later we can expand these to actually give a link to where to open a bug report or something along those lines, which I think would be nice since panics will often be development issues. Such as here, clap and cleaning the targets should handle anything users can throw at this, so any panics would be an development bug I overlooked.

I think I hit everything asked for except for two spots:

1. I didn't add frontend support for targets (such as origen.target) because I have target updates in my pattern generation branch that do some of this. origen.tester.targets was already in this though, so that's there. Maybe think of this as Target system changes #88 part 1?
2. I don't block any attempts to set the DUT multiple times. If you add falcon and eagle in that order, falcon will be instantiated first then immediately usurped by eagle. This is how interactive or other just plain scripts work, so I didn't add any extras here for that. It'll behave the same as it does in other places. We could wrap instantiate_dut in some sort of app booting stage or something to catch this if we want to though.

[coreyeng]

Hi @ginty, Is there anything else you'd like to see in this PR? I wasn't planning to do the whole pattern generation here, as that's pretty massive on its own. It'll also have the second half of #88 and #86 as well.

[ginty]

Hi @coreyeng, nah let's just go ahead and merge this if you're ready - I've kinda lost track of what's in it tbh, but your target command update looks nice!
I'm not a huge fan of printing out the full path for the activated targets, would prefer to see it just print out:

The targets currently enabled are:
dut/eagle.py
tester/sim.py
tester/v93_smt7.py

No big deal though.

[coreyeng]

Thanks! Yeah, this definitely grew a bit out of control. I'll try to do better next time.

I'll update the target command as well. The full paths ended up being a side effect of where I chose to do the name-cleansing. I'll switch it back and maybe add a --full-path option if the resolved path is desired.