Just for reference, if a power of 2 computation is in a performance critical path, using something like this would be better than a loop: http://aggregate.org/MAGIC/#Next%20Largest%20Power%20of%202 (it's not the exact same function, just subtract 1 before starting to get the same function, AFAIK.)
On Thu, Jan 15, 2009 at 4:39 PM, Eugene Loh <eugene....@sun.com> wrote: > I don't know what scope of changes require RFCs, but here's a trivial > change. > > ================================================== > > RFC: Eliminate opal_round_up_to_nearest_pow2(). > > WHAT: Eliminate the function opal_round_up_to_nearest_pow2(). > > WHY: It's poorly written. A clean rewrite would take only > one line of code, which is best written in-place rather than > making a function call. Currently, there is only one call > site, and I expect it to go away. Similar code already inlines > such computation rather than calling this obscure, unused function. > > WHERE: opal/util > > WHEN: Upon acceptance. For 1.4. > > TIMEOUT: January 30, 2009. > > ================================================== > > WHY (details): > > The function opal_round_up_to_nearest_pow2() is defined in > opal/util/pow2.c and is declared in the corresponding include > file. It returns the calling argument rounded up to the > nearest power of 2. > > This code is difficult to read. That is, it is difficult to > reason about the code's correctness or range of validity or > its behavior outside the range of validity. Meanwhile, it > offers no compelling advantage -- e.g., fast performance or > increased robustness. It is called by only one site, which > is going away. > > To use the existing function, one must know of its existence, > include the proper header file, and make the appropriate function > call. > > In contrast, a cleanly written version of this code takes only > one line of code. Hence, calling sites are cleaner and faster > if they simply in-line this computation. Further, such code is > customizable (e.g., round down to a power of 2). Most "round to > power of 2" computations in OMPI today already simply implement > this computation themselves rather than calling this obscure > function. E.g., search on "pow2" in > ompi/mca/coll/tuned/coll_tuned_decision_fixed.c. > > WHAT (details): > > - Remove the file opal/util/pow2.c. > > - Remove the file opal/util/pow2.h. > > - In opal/util/Makefile.am, remove pow2.h and pow2.c > > - In ompi/class/ompi_circular_buffer_fifo.h, if the call > to opal_round_up_to_nearest_pow2() has not already been > removed, then remove > > #include "opal/util/pow2.h" > > and replace > > size = opal_round_up_to_nearest_pow2(size_of_fifo); > > with > > size = 1; > while ( size < size_of_fifo) > size <<= 1; > > or the even more compact > > for (size = 1; size < size_of_fifo; size <<= 1); > _______________________________________________ > devel mailing list > de...@open-mpi.org > http://www.open-mpi.org/mailman/listinfo.cgi/devel > -- Tim Mattox, Ph.D. - http://homepage.mac.com/tmattox/ tmat...@gmail.com || timat...@open-mpi.org I'm a bright... http://www.the-brights.net/
