Re: [Mesa-dev] [Mesa-stable] [PATCH v2 1/2] gallivm: correct channel shift logic on big endian

[Matt Turner]

On Tue, Aug 29, 2017 at 1:13 PM, Roland Scheidegger  wrote:
> That said, I already reviewed this:
> https://lists.freedesktop.org/archives/mesa-dev/2017-August/167515.html

Since I don't think Ben has commit access, it seems that you should be
the one to commit it as well.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH v2 1/2] gallivm: correct channel shift logic on big endian

[Roland Scheidegger]

Am 29.08.2017 um 18:51 schrieb Emil Velikov:
> On 23 August 2017 at 21:32, Ben Crocker  wrote:
>> From: Ray Strode 
>>
>> lp_build_fetch_rgba_soa fetches a texel from a texture.
>> Part of that process involves first gathering the element
>> together from memory into a packed format, and then breaking
>> out the individual color channels into separate, parallel
>> arrays.
>>
>> The code fails to account for endianess when reading the packed
>> values.
>>
>> This commit attempts to correct the problem by reversing the order
>> the packed values are read on big endian systems.
>>
>> Bugzilla: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D100613=DwIBaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=-owfKrLkZG_-D07ad7TcBmuZ0jASKVGMIE-e-E9DLr0=zMHob1rOLWVOOgtwdGeaQ_qRwgTRdsKq5wrlSVRyK9o=
>>  
>> Cc: "17.2" "17.1" 
>>
>> ---
>>  src/gallium/auxiliary/gallivm/lp_bld_format_soa.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_format_soa.c 
>> b/src/gallium/auxiliary/gallivm/lp_bld_format_soa.c
>> index 98eb694..22c19b1 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_format_soa.c
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_format_soa.c
>> @@ -650,7 +650,13 @@ lp_build_fetch_rgba_soa(struct gallivm_state *gallivm,
>>   for (i = 0; i < format_desc->nr_channels; i++) {
>>  struct util_format_channel_description chan_desc = 
>> format_desc->channel[i];
>>  unsigned blockbits = type.width;
>> -unsigned vec_nr = chan_desc.shift / type.width;
>> +unsigned vec_nr;
>> +
>> +#ifdef PIPE_ARCH_BIG_ENDIAN
>> +vec_nr = (format_desc->block.bits - (chan_desc.shift + 
>> chan_desc.size)) / type.width;
>> +#else
>> +vec_nr = chan_desc.shift / type.width;
>> +#endif
>>  chan_desc.shift %= type.width;
>>
> Roland, I think you're one of the more knowledgeable people for things 
> gallivm.
> Can you please review this patch?

Well I'm the one who regularly breaks the BE stuff :-).
That said, I already reviewed this:
https://lists.freedesktop.org/archives/mesa-dev/2017-August/167515.html

Roland


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

Re: [Mesa-dev] [Mesa-stable] [PATCH v2 1/2] gallivm: correct channel shift logic on big endian

[Emil Velikov]

On 23 August 2017 at 21:32, Ben Crocker  wrote:
> From: Ray Strode 
>
> lp_build_fetch_rgba_soa fetches a texel from a texture.
> Part of that process involves first gathering the element
> together from memory into a packed format, and then breaking
> out the individual color channels into separate, parallel
> arrays.
>
> The code fails to account for endianess when reading the packed
> values.
>
> This commit attempts to correct the problem by reversing the order
> the packed values are read on big endian systems.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100613
> Cc: "17.2" "17.1" 
>
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_format_soa.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_format_soa.c 
> b/src/gallium/auxiliary/gallivm/lp_bld_format_soa.c
> index 98eb694..22c19b1 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_format_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_format_soa.c
> @@ -650,7 +650,13 @@ lp_build_fetch_rgba_soa(struct gallivm_state *gallivm,
>   for (i = 0; i < format_desc->nr_channels; i++) {
>  struct util_format_channel_description chan_desc = 
> format_desc->channel[i];
>  unsigned blockbits = type.width;
> -unsigned vec_nr = chan_desc.shift / type.width;
> +unsigned vec_nr;
> +
> +#ifdef PIPE_ARCH_BIG_ENDIAN
> +vec_nr = (format_desc->block.bits - (chan_desc.shift + 
> chan_desc.size)) / type.width;
> +#else
> +vec_nr = chan_desc.shift / type.width;
> +#endif
>  chan_desc.shift %= type.width;
>
Roland, I think you're one of the more knowledgeable people for things gallivm.
Can you please review this patch?

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