Quoting Ville Syrjälä (2019-04-10 18:59:52) > On Mon, Apr 08, 2019 at 06:05:06PM +0100, Chris Wilson wrote: > > Quoting Ville Syrjälä (2019-04-08 17:06:01) > > > On Mon, Apr 08, 2019 at 04:49:13PM +0100, Chris Wilson wrote: > > > > Quoting Ville Syrjala (2019-04-08 16:27:02) > > > > > - /* > > > > > - * Adjust the original formula to delay the division by 2^22 > > > > > in order to > > > > > - * minimize possible rounding errors. > > > > > - */ > > > > > - tmp = (u64)m1 * m2_int * ref_clock + > > > > > - (((u64)m1 * m2_frac * ref_clock) >> 22); > > > > > - tmp = div_u64(tmp, 5 * div1 * div2); > > > > > - > > > > > - return tmp; > > > > > + return div_u64(mul_u32_u32(ref_clock * m1, m2), > > > > > + (5 * div1 * div2) << 22); > > > > > > > > You say the denominator here is a u64, so do you not need to cast > > > > (u64)(5 * d1 * d2) to ensure it doesn't overflow the shift? > > > > > > It should fit into u32. The maximum value should be > > > <= (5*0xf*0x7)<<22 based on the number of bits available > > 3b * 4b * 3b = 10b. So just fits. > > > > Is it worth asserting those limits? Feels like it is running close, and > > will be subject to cargo-culting. > > I suppose checking for it might be a good idea. > > Just 'WARN_ON(5 * div1 * div2 >= 1 << 10)' maybe, or were you thinking > of something fancier?
How about something like struct { unsigned int div1 : 3; unsigned int div2 : 3; } d; then with a bit of luck smatch will spot an overflow, and people might think twice when copying? Even weirder, add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-29 (-29) Function old new delta intel_ddi_get_config 2377 2348 -29 I dread to look into the function to see how that changed gcc's mind. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx