[Mesa-dev] [PATCH] glsl: remove GCC construct from VECN macro

Roland Scheidegger sroland at vmware.com
Mon Mar 26 23:08:59 UTC 2018


Am 27.03.2018 um 00:44 schrieb Rob Clark:
> On Mon, Mar 26, 2018 at 6:38 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Mon, Mar 26, 2018 at 6:07 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>> Am 26.03.2018 um 23:01 schrieb Rob Clark:
>>>> correct me if I'm wrong, but I don't see how you could do the ## stuff
>>>> to construct the built-in type name with templates.  So I think the
>>>> options are tons of copy/pasta code (ie. what we had before) or
>>>> macros.
>>>>
>>>> I agree that could just construct the whole method with the macro.. I
>>>> like that better than changing VECN() macro to contain the return
>>>> (which I was explicitly trying to avoid in the original version, since
>>>> macros that contain return statements are evil)
>>>
>>> macros which don't compile are worse...
>>> Albeit I'm not a fan of any kind of macros, but whatever compiles.
>>
>> fair enough, although tbh gcc is the compiler I have, so I tend not to
>> realize when I introduce things that are gcc specific.  (And, well,
>> the linux kernel uses constructs like these, so I tend to assume they
>> are fair game.)
>>
>> Ian's suggestion to make the entire method body into the macro is the
>> approach I like best (and even more negative diff-stat).. I'll send a
>> patch in a few minutes.
> 
> oh, and if there is a compiler flag that would disallow things that
> intel or MS compiler doesn't like.. we should use that.  It would save
> me from embarrassing myself ;-)
> 
> (alternatively something that pro-actively pulls patches from list and
> builds them with compilers that most mesa dev's don't have, along the
> lines of what kbuild robot does, could work.. the original patch was
> sitting on list for at least a week.. but I guess the compiler arg, if
> it exists, is the easier solution.)

I don't think there is such a flag - -Wpedantic should flag some usage
of non-iso extensions but not all.
I believe clang should warn about it unless you use -Wno-gnu (or maybe
it would need -Wgnu).

Roland



> 
> BR,
> -R
> 
> 
>> BR,
>> -R
>>
>>> Roland
>>>
>>>
>>>>
>>>> BR,
>>>> -R
>>>>
>>>> On Mon, Mar 26, 2018 at 4:33 PM, Ian Romanick <idr at freedesktop.org> wrote:
>>>>> As much as I'm not a fan of C++ templates, I don't see how this
>>>>> preprocessor macro reinvention of that mechanism is an improvement.
>>>>>
>>>>> On 03/26/2018 09:45 AM, Emil Velikov wrote:
>>>>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>>>>
>>>>>> Update the macro to have the explic return. Using the current GCC
>>>>>> specific macro breaks other compilers such as the Intel one or MSVC.
>>>>>>
>>>>>> Bugzilla: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D105740&d=DwIBaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=wN0OlA9-uN-YjyUfAXoi2DgA6qDPtzMil4RM0xhwDyM&s=dvKEnjT1-nDYwicM-O11QsrqsmV6TlYiouvj0Pc1R1Q&e=
>>>>>> Fixes: f407edf3407396379e16b0be74b8d3b85d2ad7f0
>>>>>> Cc: Rob Clark <robdclark at gmail.com>
>>>>>> Cc: Timothy Arceri <tarceri at itsqueeze.com>
>>>>>> Cc: Roland Scheidegger <sroland at vmware.com>
>>>>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>>>>> ---
>>>>>>  src/compiler/glsl_types.cpp | 30 +++++++++++++++---------------
>>>>>>  1 file changed, 15 insertions(+), 15 deletions(-)
>>>>>>
>>>>>> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
>>>>>> index b8caddb406..ab356cb5a0 100644
>>>>>> --- a/src/compiler/glsl_types.cpp
>>>>>> +++ b/src/compiler/glsl_types.cpp
>>>>>> @@ -509,88 +509,88 @@ glsl_type::vec(unsigned components, const glsl_type *const ts[])
>>>>>>     return ts[n - 1];
>>>>>>  }
>>>>>>
>>>>>> -#define VECN(components, sname, vname) ({        \
>>>>>> +#define VECN(components, sname, vname)  {        \
>>>>>>        static const glsl_type *const ts[] = {     \
>>>>>>           sname ## _type, vname ## 2_type,        \
>>>>>>           vname ## 3_type, vname ## 4_type,       \
>>>>>>           vname ## 8_type, vname ## 16_type,      \
>>>>>>        };                                         \
>>>>>> -      glsl_type::vec(components, ts);            \
>>>>>> -   })
>>>>>> +      return glsl_type::vec(components, ts);     \
>>>>>> +   }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::vec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, float, vec);
>>>>>> +   VECN(components, float, vec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::f16vec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, float16_t, f16vec);
>>>>>> +   VECN(components, float16_t, f16vec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::dvec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, double, dvec);
>>>>>> +   VECN(components, double, dvec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::ivec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, int, ivec);
>>>>>> +   VECN(components, int, ivec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::uvec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, uint, uvec);
>>>>>> +   VECN(components, uint, uvec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::bvec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, bool, bvec);
>>>>>> +   VECN(components, bool, bvec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::i64vec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, int64_t, i64vec);
>>>>>> +   VECN(components, int64_t, i64vec);
>>>>>>  }
>>>>>>
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::u64vec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, uint64_t, u64vec);
>>>>>> +   VECN(components, uint64_t, u64vec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::i16vec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, int16_t, i16vec);
>>>>>> +   VECN(components, int16_t, i16vec);
>>>>>>  }
>>>>>>
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::u16vec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, uint16_t, u16vec);
>>>>>> +   VECN(components, uint16_t, u16vec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::i8vec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, int8_t, i8vec);
>>>>>> +   VECN(components, int8_t, i8vec);
>>>>>>  }
>>>>>>
>>>>>>
>>>>>>  const glsl_type *
>>>>>>  glsl_type::u8vec(unsigned components)
>>>>>>  {
>>>>>> -   return VECN(components, uint8_t, u8vec);
>>>>>> +   VECN(components, uint8_t, u8vec);
>>>>>>  }
>>>>>>
>>>>>>  const glsl_type *
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> mesa-dev mailing list
>>>>> mesa-dev at lists.freedesktop.org
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DwIBaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=wN0OlA9-uN-YjyUfAXoi2DgA6qDPtzMil4RM0xhwDyM&s=maTf5sw7O37OOchFDWU4WU8yxK7Y4pYtg7em-T3s9a8&e=
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DwIBaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=wN0OlA9-uN-YjyUfAXoi2DgA6qDPtzMil4RM0xhwDyM&s=maTf5sw7O37OOchFDWU4WU8yxK7Y4pYtg7em-T3s9a8&e=
>>>>
>>>



More information about the mesa-dev mailing list