[PATCH v2 1/3] vcomp: Implement _vcomp_for_dynamic_init_i8()

David Koller dk at cs.stanford.edu
Fri Feb 18 15:17:35 CST 2022


Thanks a lot, Zebediah, for your comments on my vcomp patches; I
really appreciate you taking time to review them!

> Does it even make sense to make these unions? Can we just make the
> relevant members 64-bit?
>
> Alternatively, should these be stored separately from the 32-bit
> versions? Tests showing what happens if you mix and match 32-bit and
> 64-bit calls might be helpful.

Right, I considered three alternatives:
1. Create new separate vcomp_thread and vcomp_task structures for the
64-bit data,
2. Increase the 32-bit data members to 64-bit, or
3. Use unions

Creating new separate 64-bit data structures for the 64-bit calls
would necessitate adding a bunch of new and redundant code (to
initialize the new structs, etc.), but would be the most performant.
Increasing the 32-bit members to 64-bit would waste the most memory
and be the least performant for 32-bit calls, but would eliminate
redundant code. My choice of unions is a compromise; new data
structures are unnecessary and the separate 32-bit and 64-bit calls
each access their native data types, which is good for performance.

Using unions in this way is a traditional approach in OpenMP runtime
implementations. For example, if you look at the code for libgomp, the
most widely used OpenMP implementation, you will see that it uses
unions to handle 32/64-bit variants in a similar manner:

   https://github.com/gcc-mirror/gcc/blob/1931cbad498e625b1e24452dcfffe02539b12224/libgomp/libgomp.h#L288-L396

>
> Could we share any (or all) of this code with the 32-bit version?
>

Yes, if the top priority was to minimize redundant code, then
refactoring these functions to handle both the 32-bit and 64-bit cases
is possible. However, performance would suffer. If you look at the
other previously-implemented vcomp functions, you'll see that earlier
Wine contributors implemented separate 32-bit and 64-bit versions of
functions for other scheduling modes: _vcomp_for_static_init() and
_vcomp_for_static_init_i8(), _vcomp_for_static_simple_init() and
_vcomp_for_static_simple_init_i8(), etc. My patches follow this same
pre-established pattern.

I believe that my patches strike a good balance between performance,
memory usage, and code concision. clarity, and consistency. If you
insist on me changing something, I'm happy to do so. Otherwise, I hope
we can get these patches committed swiftly, so that I can move my
attention to making further improvements to Wine's vcomp/OpenMP
support.



More information about the wine-devel mailing list