> 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.
> 
> Steve Reinhardt wrote:
>     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).

The thing is that these functions can be used anywhere and as long as they're 
correct, I think efficiency is generally more important than performance.  
Python's codebase has an int_pow function (to implement integer exponentiation) 
that is implemented in a manner that gabe suggests.  I think using pow() is 
definitely wrong since ieee doubles only have 52 bits of mantissa guaranteeing 
imprecise answers for numbers > 2**52


- Nathan


-----------------------------------------------------------
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

Reply via email to