[Mesa-dev] [PATCH] radeonsi: correct si_vgt_param_key on big endian machines

[Bas Vermeulen]

I am working on a new version of this patch. I have one version which does
away with all the bitfields, and uses
functions to update the index.
Another approach would be to change the union to a struct, and use a
function to get the index.
Yet another approach would be to keep the contents of the union and the
index in one struct, and use a function to
(re)calculate the index.

Which would you prefer?

Bas Vermeulen

On Tue, Mar 20, 2018 at 6:33 PM, Gert Wollny <gw.fossdev at gmail.com> wrote:

> Am Dienstag, den 20.03.2018, 15:33 +0100 schrieb Nicolai Hähnle:
> > Nice, did you actually get it to work entirely on a big endian
> > machine?
> >
> > Bit fields aren't super portable, [...]
> Indeed, the order of the bits in a bit field is compiler implementation
> dependent. To make sure that changing the compiler doesn't change the
> behaviour of the code I'd suggest that instead of using a bit field the
> index should be created by explicitly shifting the bits into the right
> positions.
>
> Best,
> Gert
>
> > However, I
> > think we should use the PIPE_ARCH_LITTLE_ENDIAN define from
> > u_endian.h
> >
> > Cheers,
> > Nicolai
> >
> > On 20.03.2018 15:21, Bas Vermeulen wrote:
> > > Using mesa OpenCL failed on a big endian PowerPC machine because
> > > si_vgt_param_key is using bitfields and a 32 bit int for an
> > > index into an array.
> > >
> > > Fix si_vgt_param_key to work correctly on both little endian
> > > and big endian machines.
> > >
> > > Signed-off-by: Bas Vermeulen <bas at daedalean.ai>
> > > ---
> > >   src/gallium/drivers/radeonsi/si_pipe.h | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > >
> > > diff --git a/src/gallium/drivers/radeonsi/si_pipe.h
> > > b/src/gallium/drivers/radeonsi/si_pipe.h
> > > index 2053dcb9fc..32dbdf6e2c 100644
> > > --- a/src/gallium/drivers/radeonsi/si_pipe.h
> > > +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> > > @@ -385,6 +385,7 @@ struct si_shader_ctx_state {
> > >    */
> > >   union si_vgt_param_key {
> > >     struct {
> > > +#if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> > >             unsigned prim:4;
> > >             unsigned uses_instancing:1;
> > >             unsigned
> > > multi_instances_smaller_than_primgroup:1;
> > > @@ -395,6 +396,18 @@ union si_vgt_param_key {
> > >             unsigned tess_uses_prim_id:1;
> > >             unsigned uses_gs:1;
> > >             unsigned _pad:32 - SI_NUM_VGT_PARAM_KEY_BITS;
> > > +#else /* __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ */
> > > +           unsigned _pad:32 - SI_NUM_VGT_PARAM_KEY_BITS;
> > > +           unsigned uses_gs:1;
> > > +           unsigned tess_uses_prim_id:1;
> > > +           unsigned uses_tess:1;
> > > +           unsigned line_stipple_enabled:1;
> > > +           unsigned count_from_stream_output:1;
> > > +           unsigned primitive_restart:1;
> > > +           unsigned multi_instances_smaller_than_primgroup:1;
> > > +           unsigned uses_instancing:1;
> > > +           unsigned prim:4;
> > > +#endif
> > >     } u;
> > >     uint32_t index;
> > >   };
> > >
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180409/1bd211fc/attachment-0001.html>