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... 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
