On Fri, Apr 17, 2015 at 6:06 AM, Rob Clark <robdcl...@gmail.com> wrote:
> On Fri, Apr 17, 2015 at 9:03 AM, Rob Clark <robdcl...@gmail.com> wrote:
>> On Fri, Apr 10, 2015 at 8:48 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
>>> Previously, this function returned the number of elements for structures
>>> and arrays and 0 for everything else. In NIR, this is almost never what
>>> you want because we also treat matricies as arrays so you have to
>>> special-case constantly. We already had a helper for this in two places
>>> and should have had one in lower_vars_to_ssa. We might as well make
>>> glsl_get_length just do the right thing in the first place.
>>>
>>> This also fixes a bug in locals_to_regs caused by not checking for the
>>> matrix case in one place.
>>
>> so, as this is, it blows up ttn, because all of a sudden vec4's become
>> length 4 instead of 0.. see below
>>
>>
>>> ---
>>> src/glsl/nir/nir_lower_locals_to_regs.c | 11 ++---------
>>> src/glsl/nir/nir_lower_var_copies.c | 24 ++----------------------
>>> src/glsl/nir/nir_lower_vars_to_ssa.c | 26 +++-----------------------
>>> src/glsl/nir/nir_types.cpp | 16 +++++++++++++++-
>>> 4 files changed, 22 insertions(+), 55 deletions(-)
>>>
[snip]
>>> diff --git a/src/glsl/nir/nir_types.cpp b/src/glsl/nir/nir_types.cpp
>>> index f0d0b46..10be6df 100644
>>> --- a/src/glsl/nir/nir_types.cpp
>>> +++ b/src/glsl/nir/nir_types.cpp
>>> @@ -103,7 +103,21 @@ glsl_get_matrix_columns(const struct glsl_type *type)
>>> unsigned
>>> glsl_get_length(const struct glsl_type *type)
>>> {
>>> - return type->length;
>>> + switch (glsl_get_base_type(type)) {
>>> + case GLSL_TYPE_STRUCT:
>>> + case GLSL_TYPE_ARRAY:
>>> + return type->length;
>>> + case GLSL_TYPE_FLOAT:
>>> + case GLSL_TYPE_INT:
>>> + case GLSL_TYPE_UINT:
>>> + case GLSL_TYPE_BOOL:
>>> + if (type->is_matrix())
>>> + return type->matrix_columns;
>>> + else
>>> + return type->vector_elements;
>>
>> So I *think* you want to return 0 instead of type->vector_elements..
>> either that or for a matrix you would want matrix_columns *
>> vector_elements? It at least seems inconsistent for a vector to
>> return number of elements, but a matrix to only return number of
>> vectors
The idea was to return whatever number of "thing"s the type has. From
NIR's perspective, since we don't have a matrix type, a matrix is a
vec4[4] so it has 4 things. An array has the length number of things,
a structure has field elements, and a vector has components. The
reason for this last one is that, if we ever want to scalarize
load/store operations, we will need vectors to also have a length.
Even there, how does it make sense for a vec4 to have a length of 0?
Should it be at least 1?
>> I'm not quite sure what you expect the array length to be for vecN's..
>> it's possible that we need to adjust expectations in ttn (and vc4 and
>> freedreno/ir3)..
>
> oh, I should add that with this locally changed to return 0 instead of
> vector_elements, on top of your other variable-indexing patches: 11
> fail->pass out of variable-indexing tests..
Glad to hear it!
--Jason
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
