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. Kristian > > So with s/~0ul/-1ll/, I think that'll work? It's all evaluated at > compile-time in any case, so clarity is the only metric. I don't have > a preference. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev