> 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. > } > > Brad Beckmann wrote: > 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.
I don't think that this has been addressed. I really don't like that this is one of the only places we use C++ exceptions in gem5. I would be quite surprised if this is truely the only way to implement this code. Any chance you could reach out to Derek on this to see if he remembers why he did it this way? However, since I don't have the initiative to suggest a better solution, I won't block the patch based on this. It's more of a style thing anyway. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2789/#review6216 ----------------------------------------------------------- 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/2789/ > ----------------------------------------------------------- > > (Updated May 26, 2015, 7:45 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10819:2f5c6078ccc8 > --------------------------- > 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/ruby/slicc_interface/AbstractController.hh > df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/slicc/ast/InPortDeclAST.py df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/slicc/ast/PeekStatementAST.py > df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/slicc/symbols/StateMachine.py > df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/slicc/symbols/Var.py df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/protocol/RubySlicc_Exports.sm > df2aa91dba5b0f0baa351039f0802baad9ed8f1d > > 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
