> On 2010-08-17 14:56:45, Ali Saidi wrote: > > src/base/intmath.hh, line 84 > > <http://reviews.m5sim.org/r/164/diff/1/?file=1817#file1817line84> > > > > How about a warning if it's called with an exponent > 20? > > > > Steve Reinhardt wrote: > I'd prefer something a little more robust. One option is to track old > and new result values in the loop, and warn if new_result < old_result. > Another option is just to do it in FP by calling pow(), then verify that the > result is something that can be safely cast back to a uint64_t. (I like that > second one, to be honest, assuming that this isn't called that frequently.) > The advantage of the first one is that if you actually do care about > performance, you could code it up in such a way that the test is only done in > debug mode. However I don't recommend that unless you're really doing this > every 10 ticks. > > Ali Saidi wrote: > I have a warning if exp > 20 and a check old_val < result on every > iteration of the loop. sound good? > > Gabe Black wrote: > I considered adding that to my example code above, but then it seemed > that the calculation could overflow over and over and spew warnings. Also, > isn't it possible to overflow but still end up with a larger value? I don't > think it would be that difficult to find a case. Getting a warning sometimes > would be nice if it catches a problem, but if it doesn't it could give a > false sense of security. The FP suggestion seems a lot safer, but then too I > imagine you could end up having to round (you will, if you have a truely 64 > bit integer) and getting something that doesn't actually overflow but that > fails the test.
If you've got the per-iteration check then I think the exp>20 warning is superfluous, right? Gabe, as long as you're checking every iteration, then I don't think there's an overflow case that would fail the test (though I could be wrong). Also, for the pow() approach, you'd expect to round, you'd just be testing that the float return value <= MAXINT (or whatever the uint64_t equivalent is). - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/164/#review210 ----------------------------------------------------------- On 2010-08-13 09:44:12, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/164/ > ----------------------------------------------------------- > > (Updated 2010-08-13 09:44:12) > > > Review request for Default. > > > Summary > ------- > > ARM: Add I/O devices for booting linux > > > Diffs > ----- > > src/base/intmath.hh 3c48b2b3cb83 > src/dev/arm/RealView.py PRE-CREATION > src/dev/arm/SConscript 3c48b2b3cb83 > src/dev/arm/Versatile.py 3c48b2b3cb83 > src/dev/arm/amba_device.hh PRE-CREATION > src/dev/arm/amba_device.cc PRE-CREATION > src/dev/arm/amba_fake.hh PRE-CREATION > src/dev/arm/amba_fake.cc PRE-CREATION > src/dev/arm/gic.hh PRE-CREATION > src/dev/arm/gic.cc PRE-CREATION > src/dev/arm/pl011.hh PRE-CREATION > src/dev/arm/pl011.cc PRE-CREATION > src/dev/arm/realview.hh PRE-CREATION > src/dev/arm/realview.cc PRE-CREATION > src/dev/arm/rv_ctrl.hh PRE-CREATION > src/dev/arm/rv_ctrl.cc PRE-CREATION > src/dev/arm/timer_sp804.hh PRE-CREATION > src/dev/arm/timer_sp804.cc PRE-CREATION > src/dev/arm/versatile.hh 3c48b2b3cb83 > src/dev/arm/versatile.cc 3c48b2b3cb83 > > Diff: http://reviews.m5sim.org/r/164/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
