> On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.hh, line 88 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51447#file51447line88> > > > > Brace on next line.
Fixed. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.hh, line 92 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51447#file51447line92> > > > > short int --> PhysRegIndex Fixed. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.hh, line 107 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51447#file51447line107> > > > > Reference The virtual function in sim_object.cc does not return a reference. Therefore, if I return const std::string&, the compiler throws an error 'returning reference to temp variable'. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.hh, line 195 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51447#file51447line195> > > > > Brace on next line. Fixed. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.hh, line 253 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51447#file51447line253> > > > > Brace on next line. Fixed. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.hh, line 276 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51447#file51447line276> > > > > Tick. A value of zero should mean no dependencies. I chose an int type for the delay instead of uint because I want to assert that it is positive. Tick is a uint. A value of zero can occur if two stores are committed in the same cycle and are annotated to have store-after-store dependency. So, delay >= 0 is valid. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.hh, line 338 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51447#file51447line338> > > > > Should the type be InstSeqNum? Fixed. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.hh, line 404 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51447#file51447line404> > > > > Is there any use case where we may not want to write all the records in > > the trace? The num_to_write arg is always equal to window size except in the last flush of the trace when it can be less than window size. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.cc, line 78 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51448#file51448line78> > > > > Will this throw an exception in case we cannot access the file? Yes it will, the protoio code handles this. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.cc, lines 104-108 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51448#file51448line104> > > > > How about always enqueuing the event? I don't see why it would be better to add overhead of scheduling an event at time 'now' and calling the event wrapped function. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.cc, line 138 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51448#file51448line138> > > > > I think this variable is not of any use. So, I agree but it protects from registering the probes more than once and it's worth it because I am anticipating use may register same listener twice. > On Nov. 6, 2015, 6:45 a.m., Nilay Vaish wrote: > > src/cpu/o3/probe/elastic_trace.cc, line 356 > > <http://reviews.gem5.org/r/3027/diff/2/?file=51448#file51448line356> > > > > I think the convention in gem5 is to write (!condition). Fixed. - Radhika ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3027/#review7518 ----------------------------------------------------------- On Nov. 19, 2015, 5:18 p.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3027/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2015, 5:18 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > The elastic trace is a type of probe listener and listens to probe points > in multiple stages of the O3CPU. The notify method is called on a probe > point typically when an instruction successfully progresses through that > stage. > > As different listener methods mapped to the different probe points execute, > relevant information about the instruction, e.g. timestamps and register > accesses, are captured and stored in temporary InstExecInfo class objects. > When the instruction progresses through the commit stage, the timing and the > dependency information about the instruction is finalised and encapsulated in > a struct called TraceInfo. TraceInfo objects are collected in a list instead > of writing them out to the trace file one a time. This is required as the > trace is processed in chunks to evaluate order dependencies and computational > delay in case an instruction does not have any register dependencies. By this > we achieve a simpler algorithm during replay because every record in the > trace can be hooked onto a record in its past. The instruction dependency > trace is written out as a protobuf format file. A second trace containing > fetch requests at absolute timestamps is written to a separate protobuf > format file. > > If the instruction is not executed then it is not added to the trace. > The code checks if the instruction had a fault, if it predicated > false and thus previous register values were restored or if it was a > load/store that did not have a request (e.g. when the size of the > request is zero). In all these cases the instruction is set as > executed by the Execute stage and is picked up by the commit probe > listener. But a request is not issued and registers are not written. > So practically, skipping these should not hurt the dependency modelling. > > If squashing results in squashing younger instructions, it may happen that > the squash probe discards the inst and removes it from the temporary > store but execute stage deals with the instruction in the next cycle which > results in the execute probe seeing this inst as 'new' inst. A sequence > number of the last processed trace record is used to trap these cases and > not add to the temporary store. > > The elastic instruction trace and fetch request trace can be read in and > played back by the TraceCPU. > > > Diffs > ----- > > src/cpu/o3/probe/elastic_trace.hh PRE-CREATION > src/cpu/o3/probe/elastic_trace.cc PRE-CREATION > src/proto/SConscript c0ea80fed78fef29ad2829b9d93e7bd568c46665 > src/proto/inst_dep_record.proto PRE-CREATION > src/proto/packet.proto c0ea80fed78fef29ad2829b9d93e7bd568c46665 > src/cpu/o3/probe/ElasticTrace.py PRE-CREATION > src/cpu/o3/probe/SConscript c0ea80fed78fef29ad2829b9d93e7bd568c46665 > > Diff: http://reviews.gem5.org/r/3027/diff/ > > > Testing > ------- > > > Thanks, > > Curtis Dunham > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
