----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2846/#review6398 -----------------------------------------------------------
src/arch/x86/isa/microops/specop.isa (line 70) <http://reviews.gem5.org/r/2846/#comment5501> Can you please set flags consistently for this instruction? Either move this up into the X86MicroopBase constructor call, or move the setFlags bits (IsNonSpeculative, IsQuiesce) down into the body here to be consistent with this line of code? I don't see any other examples of flag setting in micro-op code, but x86 macro-op code seems to follow the latter convention. src/cpu/o3/cpu.cc (line 754) <http://reviews.gem5.org/r/2846/#comment5505> Is there a reason not to include code to clear stalls within fetch.squash() and decode.squash()? The stalls[tid] arrays are only accessed within their respective pipe stages and set/reset based on signals from adjacent pipe stages. Exposing them publicly through the removeStall() functions seems like it breaks the existing pipe stage stall signalling abstraction. src/cpu/o3/fetch_impl.hh <http://reviews.gem5.org/r/2846/#comment5506> Removing this assertion seems to be a symptom of changing the abstraction for communicating stalls between pipe stages. - Joel Hestness On May 26, 2015, 4:45 p.m., Alexandru Dutu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2846/ > ----------------------------------------------------------- > > (Updated May 26, 2015, 4:45 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10858:2fb10c49dd93 > --------------------------- > cpu: o3: Merging haltContext with suspendContext > This patch improves suspendContext by flushing the pipeline, which frees > resources for other hardware threads. Secondly, it makes haltContext call > suspendContext which is not freeing architectural registers mappings on halt. > Something that haltContext previously did and was not realistic. > For example, in SMT implementations the architectural registers for a > particular hardware thread will always have mapped some physical registers and > having one thread finish execution will never create more available physical > registers for other hardware threads as there will be scheduled a different > software thread to execute on that hardware thread anyway. > As a consequence, this patch helps enabling SMT in x86 by not putting the > physical registered mapped to the ZeroRegister on the freeList for a different > thread to pick up when one of the threads has finished executing and called > exit. > > > Diffs > ----- > > src/cpu/o3/fetch_impl.hh d02b45a554b52c68cce41e1b3895fb8582a639dd > src/arch/x86/isa/microops/specop.isa > d02b45a554b52c68cce41e1b3895fb8582a639dd > src/cpu/o3/cpu.hh d02b45a554b52c68cce41e1b3895fb8582a639dd > src/cpu/o3/cpu.cc d02b45a554b52c68cce41e1b3895fb8582a639dd > src/cpu/o3/decode.hh d02b45a554b52c68cce41e1b3895fb8582a639dd > src/cpu/o3/decode_impl.hh d02b45a554b52c68cce41e1b3895fb8582a639dd > src/cpu/o3/fetch.hh d02b45a554b52c68cce41e1b3895fb8582a639dd > > Diff: http://reviews.gem5.org/r/2846/diff/ > > > Testing > ------- > > Quick regressions passed. > > > Thanks, > > Alexandru Dutu > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
