On 8/5/2018 6:58 PM, Michael Niedermayer wrote: > On Sun, Aug 05, 2018 at 06:19:01PM -0300, James Almer wrote: >> On 8/5/2018 5:29 PM, Michael Niedermayer wrote: >>> Divisions tend to be slower than shifts unless the compiler optimizes them >>> out. >>> And some of these are in inner loops. >> >> This patch makes the code slightly less readable, IMO. What compiler >> doesn't optimize out an integer division by 2 into a right shift? >> Is this part of the cause for the timeouts you mention in patch 9/9? Do >> those run on builds with compiler optimizations disabled then? > > Iam running them with -O1 > i dont know if gcc succeeded optimizing the /2 to a plain shift. (it has > to proof that the value is positive as nicolas said) > Its more a habit of not doing potentially slow operations in inner loops. > > As in "always favor shift over division when possibly" vs. "spend time > proofing that the speedloss either doesnt matter or that every supported > compiler can optimize it" > > > >> >> The other patches in this set look good since they effectively simplify >> and optimize the code for all scenarios (memcpy vs loop, reduced amount >> of iterations, etc). But this one is trying to work around a decision >> made by a compiler in a non real world scenario with specific runtime >> constrains. >> Does this timeout still happen if you apply the entire set except for >> this patch? > > Ill have to try but i dont think its wise to leave divisions in inner > loops where they are easy avoidable. > also makes greping for such suboptimal code harder, if anyone tried that > as in "divisions in a loop" > > but ye, if people prefer to leave that, we can do that (assuming it > isnt slower), should i test this ?
No, don't bother. If a compiler needs to proof the value is positive before optimizing the division into a shift then it's a different situation. Maybe make these unsigned as suggested by Nicolas instead of changing division into shift, which will also address the readability concerns, but otherwise this patch is ok. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel