> On 2011-11-28 10:29:40, Nilay Vaish wrote: > > Andreas, can you explain in more detail the functionality which is lacking > > right now and how your series of patches address it? > > Gabe Black wrote: > Yes, this is a major change and it hasn't even been mentioned before. > What does it do, how does it do it, and why is it necessary? I don't know > what you mean by "non-structural ports". Looking at your patch briefly, you > may be reinventing functional accesses. Also, in the relatively near future > I'm probably going to replace Translating and Virtual ports entirely with a > combined version that will be shared between SE and FS modes. The distinction > between those modes is on its way out, and it would be a bad idea to bake it > into class names, let alone their functionality. Without a lot of strong > justification I don't think this or it's follow on patches should be > committed. > > Ali Saidi wrote: > We've been discussing this for 6 months now with the changes various > changes Andreas has been posting. > > 15-July Changes to the gem5 memory-system (release-0.1) > 5-Aug Changes to the gem5 memory-system (release-0.2) > > At that time everyone (most notable Steve) was happy with these changes. > We've been addressing some of the comments that Steve had and have been > otherwise busy, but please don't have a knee jerk reaction to a patch until > you understand what it's trying to do. > > > Ali Saidi wrote: > A non-structure port is a port that doesn't exist for some real reason in > gem5 (e.g. a cache port), but it used for some sort of binary loading or the > like. The change removes the all of these and replaces it with a single port. > This way you can be certain that every port has a peer (one-to-one) > relationship unlike the (one-to-N) you can have now.
I'm not going to remember a few emails from July, and I'm not just going to assume everything is fine when a changes come in that modify fundamental functionality across dozens of files with very little explanation attached. If something looks wrong and I don't know otherwise, I'm going to say something. Then you can take all the time in the world to convince me otherwise, and the repository will be safe. I also still have no idea what a non-structure port is. If it doesn't exist, why does it matter? It doesn't look like anything is being removed, just turned into a different type of port with a new name. I still don't see what this is doing that's better than what's already there. Maybe once I understand that this will all make sense and be fine, but in the mean time these changes make me really uncomfortable. Since this is turning into it's own discussion, lets move over to email. Once we hash everything out we can update the commit message so people can know what happened and why. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/916/#review1693 ----------------------------------------------------------- On 2011-11-28 10:16:33, Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/916/ > ----------------------------------------------------------- > > (Updated 2011-11-28 10:16:33) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > MEM: Add the port proxies to the source tree > > Port proxies are used to replace non-structural ports, and thus enable > all ports in the system to correspond to a structural entity. > > The following replacements are going to follow in the next patches: > FunctionalPort > PortProxy > TranslatingPort > SETranslatingProxy > VirtualPort > FSTranslatingProxy > > This patch does not instantiate any of the aforementioned ports, it > merely adds the source files. > > > Diffs > ----- > > src/mem/SConscript e70d031cb5f9 > src/mem/fs_translating_proxy.hh PRE-CREATION > src/mem/fs_translating_proxy.cc PRE-CREATION > src/mem/port_proxy.hh PRE-CREATION > src/mem/port_proxy.cc PRE-CREATION > src/mem/se_translating_proxy.hh PRE-CREATION > src/mem/se_translating_proxy.cc PRE-CREATION > > Diff: http://reviews.m5sim.org/r/916/diff > > > Testing > ------- > > util/regress passing all ignoring failing eio and t1000 > > > Thanks, > > Andreas > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
