> On May 29, 2015, 12:34 a.m., Brad Beckmann wrote: > > Any more comments on the updated patch? > > Joel Hestness wrote: > Um... nothing changed in the updated patch? > > I still dislike binding implicit meaning to '*'. > > Sooraj Puthoor wrote: > With the "wildcard next state" approach, the programmer do not know the > next state while writing the protocol. So, the programmer has to write a > transition with the only information he has at that time which is the next > state can be "any state". From a programming perspective, I think '*' is the > most appropriate character to capture/represent this sentiment. Even more so > becasue '*' is widely used in programming languages as the wildcard character. > > Now, the question "which is the next state?" is resolved by the > getNextState() function during runtime. Please see my detailed response to > Jason's comment to understand how exactly this functionality can be used. > > Sooraj Puthoor wrote: > Joel, please let me know if the above explanation makes sense. If no more > concerns, please give it a ship it! > > Joel Hestness wrote: > I understand what this patch does, but I'm not a fan of binding '*' to a > specific function. Changes to the SLICC language should be extensible, and > this is not. If this wildcard-next-state operator gets into mainline code, it > will be a fair amount of work to refactor it and make it extensible. > > Also, you (Brad) asked for more comments on the 'updated' patch, but the > patch wasn't actually updated on Reviewboard. Did something actually change? > > Sooraj Puthoor wrote: > Regarding the 'updated' patch, actually the patch did not change. > Apparently, a lot of patches from "Tony Gutierrez" got reposted without any > change to those patches. Probably, some scripting issue. > > Coming back to the comments on the patch, just to clear my confusion, are > you proposing '*' symbol should be replaced with some other symbol or are you > proposing that wildcard-next-state functionality should be implemented in > some other way? And how does this change makes SLICC nonextensible and > difficult to refactor?
As I stated on the email thread: "Using '*' to designate the specific function 'getNextState()' binds implicit semantics of a getNextState function when it is present. In Sooraj's example, you'd only be able to get the next state from the TBE's nextState variable (unless you have a way to pass parameters to getNextState() somehow?). If the third transition parameter were parsed as an expr, you could put in different [added: next state] functions for different sets of "wildcard" transitions, and you'd avoid introducing any implicit function declaration semantics." In other words, this is not extensible, because binding the meaning of '*' to a specific function disallows other functions from being used to get the next state. It seems like such a situation would arise pretty quickly as soon as one starts grouping wildcard transitions. Further, this patch would need to be effectively reverted to add multiple next state functions, and we'd have to remove or deprecate uses of '*' as a wildcard operator. Nilay described what it would take to call next state functions in transition headers in lieu of this patch (also on the email thread): "There are several things need to be tackled here: a. we specify states and events in transitions as 'ident' and not as 'enum'. This is different from how we use them in rest of the code. b. variables specified in the next state function call would need to be added to the symbol table. This means that we would have to fix the set of parameters anyway." As it stands, I oppose this patch. Nice-to-have features - like wildcard next states on transitions, here - should not introduce new SLICC language constructs unless the feature cannot be implemented any other way. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2790/#review6420 ----------------------------------------------------------- On May 26, 2015, 7:45 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2790/ > ----------------------------------------------------------- > > (Updated May 26, 2015, 7:45 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10820:5064fb0968bb > --------------------------- > slicc: support for transitions with a wildcard next state > > This patches adds support for transitions of the form: > > transition(START, EVENTS, *) { ACTIONS } > > This allows a machine to collapse states that differ only in the next state > transition to collapse into one, and can help shorten/simplfy some protocols > significantly. > > When * is encountered as an end state of a transition, the next state is > determined by calling the machine-specific getNextState function. The next > state is determined before any actions of the transition execute, and > therefore the next state calculation cannot depend on any of the transition > actions. > > > Diffs > ----- > > src/mem/slicc/parser.py df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/slicc/symbols/State.py df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/slicc/symbols/StateMachine.py > df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/slicc/symbols/Transition.py > df2aa91dba5b0f0baa351039f0802baad9ed8f1d > > Diff: http://reviews.gem5.org/r/2790/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
