> On May 12, 2015, 10:02 p.m., Jason Power wrote: > > src/mem/slicc/ast/PeekStatementAST.py, line 69 > > <http://reviews.gem5.org/r/2789/diff/1/?file=45006#file45006line69> > > > > Could you explain why an exception is required here? Looking through > > the rest of the gem5 code, the only other times we use exceptions are in > > the python-c++ bindings and our string wrappers. > > > > If using the exception is really required (or if it is significantly > > easier to implement with exceptions) I don't have any problems with it, but > > it seems like there is almost always a more elegant solution. I think it's > > worth a little explanation since we so rarely use them in gem5. > > Brad Beckmann wrote: > I think exceptions are needed because SLICC creates the AST in a single > pass. I certainly don't know how to do this in another way. Derek Hower > could provide you a more detailed response. > > Nilay Vaish wrote: > I am also against using exceptions for doing actual work. Exceptions are > for error handling, not for writing code. > I think the following would work. The SLICC syntax would be: > > in_port(ResponseQueue_in, {ResponseMsg, TgtResponseMsg}, responseFromDir, > rank=3) > > The order of the message types would decide the order in which each type > is tried. > > The generated code would look like: > // Declare message > const $mtid* in_msg_ptr M5_VAR_USED; > if (check for first type) { > do the required work > } else if (check for next type) { > } else { > fatal / panic. > }
Your suggestion results in more complicated nested code. One of the benefits of this patch is it allows the SLICC developer to write the in_port logic as two separate sets of conditional logic. > On May 12, 2015, 10:02 p.m., Jason Power wrote: > > src/mem/slicc/symbols/StateMachine.py, line 957 > > <http://reviews.gem5.org/r/2789/diff/1/?file=45007#file45007line957> > > > > I think this should be panic, not fatal. It's on the border so I could > > easily be persuaded. But I think it would be useful to have an abort() > > here, not an exit(1) call. Note: [8.1 Fatal v. > > Panic](http://gem5.org/Coding_Style#Fatal_v._Panic). It's mostly that this > > error happens during runtime, not at configuration time, that makes me > > think it should be a panic. (It's also a panic below in the other case of a > > failure.) > > Brad Beckmann wrote: > I completely understand your point, but invalid protocol transitions also > invoke a fatal. > > If I change this error to panic, we should also change the invalid > transistion error. Do you agree with making them both panics? Ping??? I'm interested to get people's thoughts here. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2789/#review6216 ----------------------------------------------------------- On May 11, 2015, 10:22 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2789/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 10:22 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10846:0f9c2e771fd7 > --------------------------- > slicc: support for multiple message types on the same buffer > > This patch allows SLICC protocols to use more than one message type with a > message buffer. For example, you can declare two in ports as such: > > in_port(ResponseQueue_in, ResponseMsg, responseFromDir, rank=3) { > in_port(tgtResponseQueue_in, TgtResponseMsg, responseFromDir, rank=2) { > > > Diffs > ----- > > src/mem/protocol/RubySlicc_Exports.sm > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/ruby/slicc_interface/AbstractController.hh > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/slicc/ast/InPortDeclAST.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/slicc/ast/PeekStatementAST.py > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/slicc/symbols/StateMachine.py > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/slicc/symbols/Var.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > > Diff: http://reviews.gem5.org/r/2789/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
