On Wed, Nov 11, 2015 at 1:02 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On Monday, November 02, 2015 04:29:25 PM Matt Turner wrote:
>> The generator asserts that this is true (and presumably it's useful in
>> some optimization passes?) and the VF fs_reg constructors did this (by
>> virtue of the fact that it doesn't override what init() does).
>>
>> In the next commit, calling this constructor with brw_imm_* will
>> generate an IMM file register rather than a HW_REG, making this change
>> necessary to avoid breakage with existing uses of brw_imm_v().
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.cpp | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 91eaf61..92a9437 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -427,6 +427,12 @@ fs_reg::fs_reg(struct brw_reg reg) :
>> this->subreg_offset = 0;
>> this->reladdr = NULL;
>> this->stride = 1;
>> + if (this->file == IMM &&
>> + (this->type != BRW_REGISTER_TYPE_V &&
>> + this->type != BRW_REGISTER_TYPE_UV &&
>> + this->type != BRW_REGISTER_TYPE_VF)) {
>> + this->stride = 0;
>> + }
>> }
>>
>> bool
>>
>
> It's confusing that your subject says "Set stride to 1 for vector
> immediate types" yet your code sets stride to 0.
Right, subject fail.
> I would suggest renaming the patch to
>
> "i965/fs: Set stride correctly for immediates in fs_reg(brw_reg)"
>
> and adding some text to the commit message like:
>
> "The fs_reg() constructors for immediates set stride to 0, except for
> vector-immediates, which set stride to 1. This patch makes the fs_reg
> constructor that takes a brw_reg do likewise, so that stride is set
> correctly for cases such as fs_reg(brw_imm_v(...))."
Sure, sounds good.
> Regardless,
> Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
