accellera-official / systemc

SystemC Reference Implementation
https://systemc.org/overview/systemc/
Apache License 2.0
451 stars 145 forks source link

Remove all global sc_events #9

Closed nmeum closed 2 years ago

nmeum commented 3 years ago

The changes proposed here remove all(?) global sc_events (sc_event::none and sc_process_handle::non_event) by converting them to functions returning a reference to an sc_event in the current simulation context. This eases reset of the simulation context, as discussed in #8, as these sc_events no longer contain any reference to the initial simulation context.

A useful side effect of these changes: Previously the simulation context was created on program startup by the sc_event constructor (through the invocation of sc_get_curr_simcontext()) by the aforementioned global sc_events. With these changes applied it is created on the first invocation of sc_get_curr_simcontext().

It is currently unclear to me if it would be possible to return the same reference for sc_event::none() and sc_process_handle::non_event(), currently two different class members in the sc_simcontext are used.

To the best of my knowledge neither sc_event::none nor sc_process_handle::non_event are mandated by IEEE Std 1666-2011. For this reason, the proposed changes should not affect existing standard-conforming SystemC programs.

AndrewGoodrich commented 2 years ago

This change seems reasonable, but I would have implemented it a bit differently. I am not a big fan of having references to internal fields in sc_simcontext by methods in other classes, and I certainly would prefer those methods did not change sc_simcontext fields. My preference would be methods in sc_simcontext that return the events, creating them if necessary. An additional question is whether you want the methods in sc_event and sc_process_handle if you can just call the sc_simcontext methods. Also, as far as I can tell a single event for both none and non_event would be fine.

markfoodyburton commented 2 years ago

I will close this pull request, as @AndrewGoodrich will pick this up and rework it within the LWG.

nmeum commented 2 years ago

I will close this pull request, as @AndrewGoodrich will pick this up and rework it within the LWG.

Has there been any progress on this? Will the resulting changes be made open source again?

AndrewGoodrich commented 2 years ago

Yes, and yes. It will appear in the next release.

Andy

On Nov 11, 2021, at 8:36 AM, nmeum @.***> wrote:

I will close this pull request, as @AndrewGoodrich https://github.com/AndrewGoodrich will pick this up and rework it within the LWG.

Has there been any progress on this? Will the resulting changes be made open source again?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/accellera-official/systemc/pull/9#issuecomment-966308149, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQR2K3EAPJ3B4Y7TJBYSLULPBHLANCNFSM4RX6XFPA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.