Review Board 2.0.15


ARM: Add I/O devices for booting linux

Review Request #164 - Created Aug. 13, 2010 and submitted

Information
Ali Saidi
gem5
Reviewers
Default
ARM: Add I/O devices for booting linux

   
Posted (Aug. 13, 2010, 4:21 p.m.)



  
src/base/intmath.hh (Diff revision 1)
 
 
This isn't particularly efficient. A better algorithm would be to square n over and over and multiply it in if the appropriate bit in e is set. I'm surprised there isn't already a standard library function to do this, but I wasn't able to find one quickly with Google.

Also, by setting result = 1, e == 0 doesn't need to be a special case.
  1. It depends on what the typical exponents are... looking at where this is used, I expect them to be very small.  In that case, I prefer the simplicity of this approach.
    
    There is a pow() function in the C library for doubles, but I doubt that it would be more efficient with the conversion overhead.
    
    I suggest adding a warn() when this overflows, since that would be easy to do.
Posted (Aug. 14, 2010, 12:28 a.m.)



  
src/dev/arm/timer_sp804.cc (Diff revision 1)
 
 
Is this right?  What are the possible values of control.timerPrescale?  I don't think there are many integer powers of 16 that generate valid shift amounts...
Posted (Aug. 17, 2010, 7:56 a.m.)



  
src/base/intmath.hh (Diff revision 1)
 
 
How about a warning if it's called with an exponent > 20?
  1. 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.
  2. I have a warning if exp > 20 and a check old_val < result on every iteration of the loop. sound good?
  3. 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.
  4. 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).
  5. 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
src/dev/arm/timer_sp804.cc (Diff revision 1)
 
 
You're right... this is what I get for trying to be cute... 

time = time / clock / power(16, control.timerPrescale); is what I'm after 
Posted (Aug. 17, 2010, 11:38 a.m.)



  
src/base/intmath.hh (Diff revision 1)
 
 
There is an overflow that wouldn't be caught. If n >= 18446744073709551615 the overflow wouldn't be caught. That probably isn't a reasonable case though... If you would prefer, n could be a 32bit int which would take care of the problem.