On Thu, 2015-11-19 at 12:08 -0800, Matt Turner wrote: > On Thu, Nov 19, 2015 at 11:35 AM, Kristian Høgsberg <k...@bitplanet.net> > wrote: > > On Thu, Nov 19, 2015 at 11:24 AM, Matt Turner <matts...@gmail.com> wrote: > >> On Thu, Nov 19, 2015 at 9:30 AM, Kristian Høgsberg <k...@bitplanet.net> > >> wrote: > >>> On Thu, Nov 19, 2015 at 2:05 AM, Iago Toral Quiroga <ito...@igalia.com> > >>> wrote: > >>>> From: Connor Abbott <connor.w.abb...@intel.com> > >>>> > >>>> If we tried to get/set something that was exactly 64 bits, we would > >>>> try to do (1 << 64) - 1 to calculate the mask which doesn't give us all > >>>> 1's like we want. > >>>> > >>>> v2 (Iago) > >>>> - Replace ~0 by ~0ull > >>>> - Removed unnecessary parenthesis > >>>> > >>>> Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> > >>>> --- > >>>> src/mesa/drivers/dri/i965/brw_inst.h | 6 ++++-- > >>>> 1 file changed, 4 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/src/mesa/drivers/dri/i965/brw_inst.h > >>>> b/src/mesa/drivers/dri/i965/brw_inst.h > >>>> index 4ed95c4..ec08194 100644 > >>>> --- a/src/mesa/drivers/dri/i965/brw_inst.h > >>>> +++ b/src/mesa/drivers/dri/i965/brw_inst.h > >>>> @@ -694,7 +694,8 @@ brw_inst_bits(const brw_inst *inst, unsigned high, > >>>> unsigned low) > >>>> high %= 64; > >>>> low %= 64; > >>>> > >>>> - const uint64_t mask = (1ull << (high - low + 1)) - 1; > >>>> + const uint64_t mask = (high - low == 63) ? ~0ull : > >>>> + (1ull << (high - low + 1)) - 1; > >>> > >>> Can we do > >>> > >>> const uint64_t mask = (~0ul >> (64 - (high - low + 1))); > >>> > >>> instead? > >> > >> I don't think so, because ~0ul is of type unsigned, so right shifting > >> it shifts in zeros. I was going to make a similar comment on the > >> original patch -- "-1" is preferable over ~0u with an increasingly > >> long sequence of l's because it's signed, so it's sign extended to > >> fill whatever you assign it to. In your code though, since it's an > >> operand we'd need -1ll, I think... > > > > No, shifting in zeros is the whole point. We start out with 64 1 bits, > > then shift it down enough that we end up with (high - low + 1) 1 bits > > at the bottom, which is what we're trying to compute. > > Doh. Of course. :) >
Thanks Kristian, I'll use your version and add your Rb and Matt's to it. Iago _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
