[Mesa-dev] [PATCH 11/20] i965/fs: Append uniform entries to the gather table

[Matt Turner]

On Fri, Oct 23, 2015 at 12:38 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>> On Fri, Oct 23, 2015 at 11:23 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> Abdiel Janulgue <abdiel.janulgue at linux.intel.com> writes:
>>>
>>>> This patch generates the gather table entries for ordinary uniforms
>>>> if they are present. The uniform constants here will later be packed
>>>> together with UBO constants.
>>>>
>>>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
>>>> ---
>>>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 18 +++++++++++++++++-
>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>> index d240371..e39d821 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>> @@ -1863,8 +1863,11 @@ fs_visitor::assign_constant_locations()
>>>>        }
>>>>     }
>>>>
>>>> -   stage_prog_data->nr_params = num_push_constants;
>>>>     stage_prog_data->nr_pull_params = num_pull_constants;
>>>> +   stage_prog_data->nr_params = 0;
>>>
>>> I doubt it's useful to set this to zero here only to calculate it again
>>> below.
>>>
>>>> +
>>>> +   unsigned const_reg_access[uniforms];
>>>
>>> VLAs are a non-standard extension to C++.  Usually I'd recommend to use
>>> the C++ standard library instead, but because it's banned from the i965
>>> back-end I guess the best you can get is new[]/delete[].
>>
>> So... why not just use the extension to C++? Does some compiler we
>> care about not support them?
>>
>> GCC does: https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html
>>
>> Looks like this use would be allowed by clang as well:
>> http://clang.llvm.org/compatibility.html#vla
>
> I believe only GCC has a more or less reasonable implementation, Clang's
> implementation doesn't allow much of what GCC's allows, like objects
> with user-defined constructors or anything non-POD (seems mainly
> intended for compatibility with C99 code).  MS VC++ doesn't support it
> at all.

Okay, so the usage in this patch seems fine since we don't care about
MSVC and Clang's support should be sufficient to handle an array of
unsigned.

> The C99 VLA feature was BTW made optional again in C11, and the similar
> feature proposed for C++14 (runtime-sized arrays) was rejected by the
> standards committee.  Anyway it doesn't seem like a big loss for C++,
> there are already standards-compliant ways to get a type-safe array of
> automatic duration and dynamically determined size, which support
> non-POD C++ objects as element type and behave as first-class C++
> objects themselves unlike in the various VLA-like language extensions.

Interesting. Thanks for the information.