[Mesa-dev] [PATCH v4 106/129] nir: Rework opt_copy_prop_vars to use deref instructions
Jason Ekstrand
jason at jlekstrand.net
Fri Jun 22 20:00:05 UTC 2018
On June 22, 2018 11:52:54 Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Friday, June 22, 2018 8:28:47 AM PDT Jason Ekstrand wrote:
>> On Thu, Jun 21, 2018 at 1:03 AM, Kenneth Graunke <kenneth at whitecape.org>
>> wrote:
>>
>>> On Thursday, May 31, 2018 10:06:47 PM PDT Jason Ekstrand wrote:
>>> [snip]
>>>> @@ -529,57 +509,55 @@ static bool
>>>> load_from_deref_entry_value(struct copy_prop_var_state *state,
>>>> struct copy_entry *entry,
>>>> nir_builder *b, nir_intrinsic_instr *intrin,
>>>> - nir_deref_var *src, struct value *value)
>>>> + nir_deref_instr *src, struct value *value)
>>>> {
>>>> *value = entry->src;
>>>>
>>>> - /* Walk the deref to get the two tails and also figure out if we
>>> need to
>>>> - * specialize any wildcards.
>>>> - */
>>>> - bool need_to_specialize_wildcards = false;
>>>> - nir_deref *entry_tail = &entry->dst->deref;
>>>> - nir_deref *src_tail = &src->deref;
>>>> - while (entry_tail->child && src_tail->child) {
>>>> - assert(src_tail->child->deref_type ==
>>> entry_tail->child->deref_type);
>>>> - if (src_tail->child->deref_type == nir_deref_type_array) {
>>>> - nir_deref_array *entry_arr = nir_deref_as_array(entry_tail-
>>>> child);
>>>> - nir_deref_array *src_arr = nir_deref_as_array(src_tail->
>>> child);
>>>> -
>>>
>>> I think there might be a bug here...note this condition...
>>>
>>>> - if (src_arr->deref_array_type != nir_deref_array_type_wildcard
>>> &&
>>>> - entry_arr->deref_array_type ==
>>> nir_deref_array_type_wildcard)
>>>
>>> Old: Source NOT wildcard, dest is wildcard.
>>>
>>>> - need_to_specialize_wildcards = true;
>>>> - }
>>>> + b->cursor = nir_instr_remove(&intrin->instr);
>>>>
>>>> - entry_tail = entry_tail->child;
>>>> - src_tail = src_tail->child;
>>>> + nir_deref_path entry_dst_path, src_path;
>>>> + nir_deref_path_init(&entry_dst_path, entry->dst, state->mem_ctx);
>>>> + nir_deref_path_init(&src_path, src, state->mem_ctx);
>>>> +
>>>> + bool need_to_specialize_wildcards = false;
>>>> + nir_deref_instr **entry_p = &entry_dst_path.path[1];
>>>> + nir_deref_instr **src_p = &src_path.path[1];
>>>> + while (*entry_p && *src_p) {
>>>> + nir_deref_instr *entry_tail = *entry_p++;
>>>> + nir_deref_instr *src_tail = *src_p++;
>>>> +
>>>> + if (src_tail->deref_type == nir_deref_type_array &&
>>>> + entry_tail->deref_type == nir_deref_type_array_wildcard)
>>>
>>> New: Source IS wildcard, dest is wildcard. I think you want != on the
>>> source condition to match the old behavior.
>>
>> I don't think there's a bug here. With the old mechanism, we had
>> deref_type_array and then within that deref_array_type_direct, _indirect,
>> and _wildcard. In the new scheme, we have deref_type_array and
>> deref_type_array_wildcard. So deref_type == nir_deref_type_array in the
>> new translates to deref_type == nir_deref_type_array && deref_array_type !=
>> nir_deref_array_type_wildcard. Does that make sense?
>
> I missed that it also changed from != wildcard to == array. :(
No worries. Thanks for double checking!
--Jason
More information about the mesa-dev
mailing list