----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1007/#review2048 -----------------------------------------------------------
I do not disagree with the spirit of this patch. In fact I think I advocated for it. Concretely there are some minor style issues to straighten out, and a function that could be simplified or eliminated. I don't see anything wrong with what you're actually doing, but the squashing logic for O3 makes my head hurt and I won't pretend to understand all of it. If you've tested it thoroughly then that's all I'd be able to do in your shoes. src/cpu/o3/fetch_impl.hh <http://reviews.gem5.org/r/1007/#comment2532> break on the next line please. src/cpu/o3/rob.hh <http://reviews.gem5.org/r/1007/#comment2533> Judging by the comment, this isn't really getting the squash inst, it just happens to be used that way. What it's really doing is getting an instruction with a particular sequence number, right? src/cpu/o3/rob_impl.hh <http://reviews.gem5.org/r/1007/#comment2535> This whole function could be replaced by (or perhaps wrap) one of the STL searching functions. If there isn't one that's a member of whatever type the instruction list is, then I'm pretty sure there's one in the generic algorithms stuff. Unfortunately I couldn't tell you off hand what it's called or where to find it, but I'm confident it's in there because I think I've used it before. It might even use some fancy schmancy algorithm that would be faster than this for loop, but if it's a linked list likely not. src/cpu/o3/rob_impl.hh <http://reviews.gem5.org/r/1007/#comment2534> Brace at the end of the previous line please. - Gabe Black On Jan. 28, 2012, 9 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1007/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2012, 9 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 8731:10a06bd36839 > --------------------------- > O3 CPU: Provide the squashing instruction > This patch adds a function to the ROB that will get the squashing instruction > from the ROB's list of instructions. This squashing instruction is used for > figuring out the macroop from which the fetch stage should fetch the microops. > Further, a check has been added that if the instructions are to be fetched > from the cache maintained by the fetch stage, then the data in the cache > should > be valid and the PC of the thread being fetched from is same as the address of > the cache block. > > > Diffs > ----- > > src/cpu/o3/commit_impl.hh 9d7c1dc54954 > src/cpu/o3/fetch_impl.hh 9d7c1dc54954 > src/cpu/o3/rob.hh 9d7c1dc54954 > src/cpu/o3/rob_impl.hh 9d7c1dc54954 > > Diff: http://reviews.gem5.org/r/1007/diff/diff > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
