> On July 26, 2012, 6:08 a.m., Nathan Binkert wrote: > > src/base/types.hh, line 55 > > <http://reviews.gem5.org/r/1320/diff/1/?file=28245#file28245line55> > > > > Are we sure that this is no longer the case? > > Andreas Hansson wrote: > All the regressions work :), if someone knows what the issue was I'd be > keen to know more. > > Steve Reinhardt wrote: > I don't know for sure, but it could be a matter of doing a subtraction > and checking for a negative result... however this code here looks safe, so > maybe that's been taken care of: > http://repo.gem5.org/gem5/file/b57966a6c512/src/mem/cache/tags/lru.cc#l130 > > Andreas Hansson wrote: > If there is a specific test to run let me know. As mentioned, all > regressions pass. > > Steve Reinhardt wrote: > Sorry not to be clearer... my point was that I don't see any code that > should have a problem with unsigned ticks. My hypothesis is that at some > point in the past this code I'm pointing to: > > if (blk->whenReady > curTick() > && blk->whenReady - curTick() > hitLatency) { > lat = blk->whenReady - curTick(); > } > > may have been written something more like: > > Tick delay = blk->whenReady - curTick(); > lat = (delay < 0) ? 0 : delay; > > which works fine as long as Tick is signed but breaks when it's unsigned. > But since I don't see any code like that right now, I'm cautiously > optimistic that this comment is obsolete. >
It would be good to remove the "-Wno-sign-compare", but I am afraid that would take quite some effort. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1320/#review3144 ----------------------------------------------------------- On July 26, 2012, 6:51 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1320/ > ----------------------------------------------------------- > > (Updated July 26, 2012, 6:51 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9133:93d82aae1592 > --------------------------- > Clock: Make Tick unsigned and remove UTick > > This patch makes the Tick unsigned and removes the UTick typedef. The > ticks should never be negative, and there was only one major issue > with removing it, caused by the o3 CPU using a -1 as an initial value. > > The patch has no impact on any regressions. > > > Diffs > ----- > > src/base/types.hh b57966a6c512 > src/cpu/o3/cpu.cc b57966a6c512 > src/sim/eventq.hh b57966a6c512 > > Diff: http://reviews.gem5.org/r/1320/diff/ > > > Testing > ------- > > util/regress all passing (disregarding t1000 and eio) > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
