Re: [Mesa-dev] [PATCH 1/2] intel/compiler: grf127 can not be dest when src and dest overlap in send

[Chema Casanova]

On 15/04/18 08:55, Matt Turner wrote:
> On Wed, Apr 11, 2018 at 7:30 PM, Jose Maria Casanova Crespo
>  wrote:
>> Implement at brw_eu_validate the restriction from Intel Broadwell PRM, vol 
>> 07,
>> section "Instruction Set Reference", subsection "EUISA Instructions", Send
>> Message (page 990):
>>
>> "r127 must not be used for return address when there is a src and dest 
>> overlap
>> in send instruction."
>>
>> Cc: Jason Ekstrand 
>> Cc: Matt Turner 
>> ---
>>  src/intel/compiler/brw_eu_validate.c | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/src/intel/compiler/brw_eu_validate.c 
>> b/src/intel/compiler/brw_eu_validate.c
>> index d3189d1ef5e..0d711501303 100644
>> --- a/src/intel/compiler/brw_eu_validate.c
>> +++ b/src/intel/compiler/brw_eu_validate.c
>> @@ -261,6 +261,15 @@ send_restrictions(const struct gen_device_info *devinfo,
>>brw_inst_src0_da_reg_nr(devinfo, inst) < 112,
>>"send with EOT must use g112-g127");
>>}
> 
> Put a newline here

Fixed locally.

>> +  if (devinfo->gen >= 8) {
>> + ERROR_IF(!dst_is_null(devinfo, inst) &&
>> +  (brw_inst_dst_da_reg_nr(devinfo, inst) +
>> +   brw_inst_rlen(devinfo, inst) > 127 ) &&
> 
> Remove the extra space after 127

Fixed locally.

>> +  (brw_inst_src0_da_reg_nr(devinfo, inst) +
>> +   brw_inst_mlen(devinfo, inst) >
>> +   brw_inst_dst_da_reg_nr(devinfo, inst)),
>> +  "r127 can not be dest when src and dest overlap in send");
> 
> I'd change the message to more closely match the docs:
> 
> "r127 must not be used for return address when there is a src and dest 
> overlap"
> 
> Thank you for extending the validator!
> 
> Reviewed-by: Matt Turner 

Thank you for the review.

Chema
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] intel/compiler: grf127 can not be dest when src and dest overlap in send

[Matt Turner]

On Wed, Apr 11, 2018 at 7:30 PM, Jose Maria Casanova Crespo
 wrote:
> Implement at brw_eu_validate the restriction from Intel Broadwell PRM, vol 07,
> section "Instruction Set Reference", subsection "EUISA Instructions", Send
> Message (page 990):
>
> "r127 must not be used for return address when there is a src and dest overlap
> in send instruction."
>
> Cc: Jason Ekstrand 
> Cc: Matt Turner 
> ---
>  src/intel/compiler/brw_eu_validate.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/intel/compiler/brw_eu_validate.c 
> b/src/intel/compiler/brw_eu_validate.c
> index d3189d1ef5e..0d711501303 100644
> --- a/src/intel/compiler/brw_eu_validate.c
> +++ b/src/intel/compiler/brw_eu_validate.c
> @@ -261,6 +261,15 @@ send_restrictions(const struct gen_device_info *devinfo,
>brw_inst_src0_da_reg_nr(devinfo, inst) < 112,
>"send with EOT must use g112-g127");
>}

Put a newline here

> +  if (devinfo->gen >= 8) {
> + ERROR_IF(!dst_is_null(devinfo, inst) &&
> +  (brw_inst_dst_da_reg_nr(devinfo, inst) +
> +   brw_inst_rlen(devinfo, inst) > 127 ) &&

Remove the extra space after 127

> +  (brw_inst_src0_da_reg_nr(devinfo, inst) +
> +   brw_inst_mlen(devinfo, inst) >
> +   brw_inst_dst_da_reg_nr(devinfo, inst)),
> +  "r127 can not be dest when src and dest overlap in send");

I'd change the message to more closely match the docs:

"r127 must not be used for return address when there is a src and dest overlap"

Thank you for extending the validator!

Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev