On Tue, Jul 14, 2015 at 6:02 AM, Francisco Jerez <curroje...@riseup.net> wrote:
> Connor Abbott <cwabbo...@gmail.com> writes:
>
>> sources with file == HW_REG get all their information from the
>> fixed_hw_reg field, so we need to get the stride and type from there
>> when computing the size.
>>
>> Signed-off-by: Connor Abbott <connor.w.abb...@intel.com>
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.cpp | 24 ++++++++++++++++++------
>> 1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 38b9095..64f093b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -696,24 +696,36 @@ fs_inst::regs_read(int arg) const
>> break;
>> }
>>
>> + unsigned stride;
>> + enum brw_reg_type type;
>> +
>> switch (src[arg].file) {
>> case BAD_FILE:
>> case UNIFORM:
>> case IMM:
>> return 1;
>> +
>> case GRF:
>> + stride = src[arg].stride;
>> + type = src[arg].type;
>> + break;
>> +
>> case HW_REG:
>> - if (src[arg].stride == 0) {
>> - return 1;
>> - } else {
>> - int size = components * this->exec_size * type_sz(src[arg].type);
>> - return DIV_ROUND_UP(size * src[arg].stride, 32);
>> - }
>> + stride = src[arg].fixed_hw_reg.hstride;
>> + type = src[arg].fixed_hw_reg.type;
>> + break;
>> +
>> case MRF:
>> unreachable("MRF registers are not allowed as sources");
>> default:
>> unreachable("Invalid register file");
>> }
>> +
>> + if (stride == 0)
>> + return 1;
>> +
>> + int size = components * this->exec_size * type_sz(type);
>> + return DIV_ROUND_UP(size * stride, 32);
>
> I don't think this will work unfortunately, brw_reg::hstride is the log2
> of the actual stride, unlike fs_reg::stride. Did I already mention I'm
> appalled by the fact that fs_reg has a number of fields with overlapping
> semantics but different representation, one or the other being
> applicable depending on the occasion. I guess it would be more or less
> bearable if these data members were declared private and some reasonable
> abstraction was provided to access them.
I don't think anybody's happy with it, but refactoring that is it's
own can of worms.
>
> How do you like the attached patch? It doesn't solve the fundamental
> problem but it seems to improve the situation slightly.
It seems fine to me... I was more paranoid about getting the type from
the fixed_hw_reg too, but brw_reg_from_fs_reg() in the generator we
have:
assert(reg->type == reg->fixed_hw_reg.type);
so it seems my paranoia wasn't justified. I'd like someone else who's
more experienced to take a look though. I suspect that others might
want to bikeshed about the name, but I don't have a better suggestion.
>
>> }
>>
>> bool
>> --
>> 2.4.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
