[Mesa-dev] [PATCH 01/21] intel/defines: Explicitly cast to uint32_t in SET_FIELD and SET_BITS

[Lionel Landwerlin]

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.landwerlin at 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;                                        \
>      })