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

[Bas Vermeulen]

On Mon, Apr 9, 2018 at 11:19 PM, Gert Wollny <gw.fossdev at gmail.com> wrote:

> Am Montag, den 09.04.2018, 14:03 -0400 schrieb Marek Olšák:
> > On Mon, Apr 9, 2018 at 10:51 AM, Bas Vermeulen <bas at daedalean.ai>
> > wrote:
> Which solution is better depends on what is done more often: reading
> the index or writing to the bit fields.
>

The bitfields are read and written, and the index is mostly read. I found
four instances of the bitfields being written after which the index needs to
be updated.


> > > 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.
> This emulates the code the compiler would create, but it requires that
> for each bit field setters (and getters?) must be implemented.
>

Yes. I have a git branch with this change ready if that's what's
wanted/needed.


> > > Another approach would be to change the union to a struct, and use
> > > a function to get the index.
> This method has the advantage that only the access to the index needs
> new implementation.
>

I can prepare a patch for this as well.


> > > 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.
> I don't think that would make much sense.
>

It adds four lines to the code, all the key->u.xxx has it's u. removed.
But future implementation needs to remember to call that function if any of
the bitfields are changed. Which can be annoying.

There is another option: Check at configuration time whether the bit
> field layout is like the low or the high endian layout you already
> implemented, and instead of basing the selection of the struct layout
> on the big/low-endianess of the architecture, base it on this test.
>
> It would probably be prudent to test both layouts and then fail
> configuration if non of the two reflect the actual layout (at which
> point one would have to thing about how to implement all the bit
> shifting properly).


Or just keep the union dependent on endianness, and add an assert/check/test
to make sure that everything works as expected.


> > >
> > > Which would you prefer?
> > >
> >
> > I don't mind bitfields. They make the code nice and tiny. Shifts
> > would decrease readability.
> The problem is, that the layout of bitfields is compiler dependend.
>

Let me know what you guys want to have this included. I just want it fixed,
I don't really care on the form. :)

Bas Vermeulen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180410/3bab54ff/attachment.html>