On 17/11/2018 02:47, Jason Ekstrand wrote:
If you pass a bool in as the value to set, the C standard says that it gets converted to an int prior to shifting. If you try to set a bool to bit 31, this lands you in undefined behavior. It's better just to add the explicit cast and let the compiler delete it for us.
I would use BITFIELD_BIT, but not a big deal. Reviewed-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
--- src/intel/compiler/brw_eu_defines.h | 4 ++-- src/mesa/drivers/dri/i965/brw_defines.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/intel/compiler/brw_eu_defines.h b/src/intel/compiler/brw_eu_defines.h index 52957882b10..025a459760c 100644 --- a/src/intel/compiler/brw_eu_defines.h +++ b/src/intel/compiler/brw_eu_defines.h @@ -41,14 +41,14 @@ /* Using the GNU statement expression extension */ #define SET_FIELD(value, field) \ ({ \ - uint32_t fieldval = (value) << field ## _SHIFT; \ + uint32_t fieldval = (uint32_t)(value) << field ## _SHIFT; \ assert((fieldval & ~ field ## _MASK) == 0); \ fieldval & field ## _MASK; \ })#define SET_BITS(value, high, low) \({ \ - const uint32_t fieldval = (value) << (low); \ + const uint32_t fieldval = (uint32_t)(value) << (low); \ assert((fieldval & ~INTEL_MASK(high, low)) == 0); \ fieldval & INTEL_MASK(high, low); \ }) diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h index 897c91aa31e..d7a41c83ee8 100644 --- a/src/mesa/drivers/dri/i965/brw_defines.h +++ b/src/mesa/drivers/dri/i965/brw_defines.h @@ -38,7 +38,7 @@ /* Using the GNU statement expression extension */ #define SET_FIELD(value, field) \ ({ \ - uint32_t fieldval = (value) << field ## _SHIFT; \ + uint32_t fieldval = (uint32_t)(value) << field ## _SHIFT; \ assert((fieldval & ~ field ## _MASK) == 0); \ fieldval & field ## _MASK; \ })
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
