January 2022 - Wine-Devel - list.winehq.org

[PATCH vkd3d v2] build: Disable crosstests as well if tests are disabled.

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- v2: Actually disable them in the makefile. Makefile.am | 10 ++++++++++ configure.ac | 9 +++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index 3b2e7d718..3c3858e8b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -381,6 +381,9 @@ CLEANFILES += include/private/vkd3d_version.h dummy-vkd3d-version: ## Cross-compile tests + +if BUILD_CROSSTESTS + cross_implibs = crosslibs/d3d12 CROSS_CPPFLAGS = -I$(srcdir)/include -I$(srcdir)/include/private -I$(builddir)/include CROSS_CFLAGS = -g -O2 -Wall -municode ${CROSS_CPPFLAGS} -D__USE_MINGW_ANSI_STDIO=0 @@ -440,6 +443,13 @@ else crosstest64: endif +else + +crosstest32: +crosstest64: + +endif # BUILD_CROSSTESTS + .PHONY: crosstest crosstest32 crosstest64 crosstest: crosstest32 crosstest64 diff --git a/configure.ac b/configure.ac index d296dfd4b..b13858d2d 100644 --- a/configure.ac +++ b/configure.ac @@ -143,13 +143,14 @@ AM_CONDITIONAL([BUILD_TESTS], [test "x$enable_tests" != "xno"]) AM_CONDITIONAL([HAVE_WIDL], [test "x$WIDL" != "xno"]) AM_CONDITIONAL([HAVE_CROSSTARGET32], [test "x$CROSSTARGET32" != "xno"]) AM_CONDITIONAL([HAVE_CROSSTARGET64], [test "x$CROSSTARGET64" != "xno"]) +AM_CONDITIONAL([BUILD_CROSSTESTS], + [test "x$enable_tests" != "xno" -a $ "x$CROSSTARGET32" != "xno" -o "x$CROSSTARGET64" != "xno" $]) AC_CONFIG_FILES([Makefile]) AC_OUTPUT dnl Output configuration summary -AS_IF([test "x$CROSSTARGET32" != "xno" -o "x$CROSSTARGET64" != "xno"], - [HAVE_CROSSTEST=yes], [HAVE_CROSSTEST=no]) +AM_COND_IF([BUILD_CROSSTESTS],[HAVE_CROSSTEST=yes], [HAVE_CROSSTEST=no]) AS_IF([test $DX_FLAG_doc = 1], [HAVE_DOCS=yes], [HAVE_DOCS=no]) AS_ECHO([" @@ -166,6 +167,6 @@ AS_ECHO([" Building documentation: ${HAVE_DOCS} Building crosstests: ${HAVE_CROSSTEST}"]) -AS_IF([test "x$CROSSTARGET32" != "xno"], [AS_ECHO([" Using 32-bit cross compiler: $CROSSCC32"])]) -AS_IF([test "x$CROSSTARGET64" != "xno"], [AS_ECHO([" Using 64-bit cross compiler: $CROSSCC64"])]) +AS_IF([test "$HAVE_CROSSTEST" == "yes" -a "x$CROSSTARGET32" != "xno"], [AS_ECHO([" Using 32-bit cross compiler: $CROSSCC32"])]) +AS_IF([test "$HAVE_CROSSTEST" == "yes" -a "x$CROSSTARGET64" != "xno"], [AS_ECHO([" Using 64-bit cross compiler: $CROSSCC64"])]) AS_ECHO([]) -- 2.34.1

Jan. 26, 2022

Re: [PATCH vkd3d] build: Disable crosstests as well if tests are disabled.

by Zebediah Figura (she/her)

On 1/26/22 01:54, Giovanni Mascellani wrote: > Hi, > > does this patch change anything other than what is printed at the end of > the configuration step? It seems to me that the only variable that > changes with this commit is HAVE_CROSSTEST, and I can't see where that > is used other than at the end of configure.ac. Indeed it does not; I incorrectly assumed that HAVE_CROSSTEST was not used by Makefile.am (and in fact failed to notice that it was not defined as an AM conditional). I'll resend this patch with that fixed.

Jan. 26, 2022

[PATCH] ws2_32: Increased MAX_SOCKETS_PER_PROCESS to 256

by Andrew Miskell

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50352 Signed-off-by: Andrew Miskell <andrewmiskell(a)mac.com> --- dlls/ws2_32/socket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index fe7e4fb3464..004e386e5a8 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -419,7 +419,7 @@ static BOOL socket_list_remove( SOCKET socket ) return FALSE; } -#define MAX_SOCKETS_PER_PROCESS 128 /* reasonable guess */ +#define MAX_SOCKETS_PER_PROCESS 256 /* reasonable guess */ #define MAX_UDP_DATAGRAM 1024 static INT WINAPI WSA_DefaultBlockingHook( FARPROC x ); -- 2.32.0 (Apple Git-132)

Jan. 26, 2022

Re: [PATCH] winevulkan: Enable VK_EXT_pipeline_creation_feedback.

by Georg Lehmann

On 26.01.22 17:37, Henri Verbeet wrote: > On Wed, 26 Jan 2022 at 17:35, Georg Lehmann <dadschoorse(a)gmail.com> wrote: >> >> We did previously not support this extension because it runs into 32bit >> alignment issues. But it is core in 1.3, so we already enabled the >> functionality. Additionally vkd3d-proton wants to use the extension and it >> does not care about 32bit. >> > Shouldn't that have blocked 1.3 support instead? Maybe. 32bit winevulkan has a ton of small issues, only exposing a non-broken subset is not really possible. E.g. VkPhysicalDeviceVulkan11Properties is also broken due to alignment and we don't ignore invalid pointers in a lot of situations where we should. We would not even be conformant for 1.0 without any extensions. I think the best policy is to make everything work that is actually needed, there aren't a ton of 32bit Vulkan applications. It's mostly dxvk and possibly wined3d in the future as far as I understand. The issue with VK_EXT_pipeline_creation_feedback will result in a misreported compile time duration, which is pretty minor in my opinion.

Jan. 26, 2022

Re: [PATCH v3 07/13] loader: Don't clobber existing memory mappings when reserving addresses.

by Jinoh Kang

On 1/26/22 18:06, Alexandre Julliard wrote: > Jinoh Kang <jinoh.kang.kr(a)gmail.com> writes: > >> On 1/26/22 00:48, Alexandre Julliard wrote: >>> Jinoh Kang <jinoh.kang.kr(a)gmail.com> writes: >>> >>>> Today, the preloader makes no attempt to avoid unmapping existing >>>> memory mappings except the initial stack. This results in irrevocably >>>> unmapping some useful preallocated memory areas, such as vDSO. >>>> >>>> Fix this by reading /proc/self/maps for existing VMAs, and splitting >>>> mmap() calls to avoid erasing existing memory mappings. >>> >>> That defeats the purpose of using the preloader. >> >> The intention was to *incrementally* scrape memory areas for the reserved ranges, >> relocating any critical areas (vDSO, stack, ...) along the way. >> >> It's also why this change is useless without the subsequent patches, >> which calls map_reserve_preload_ranges again to actually fill out all >> the gaps previously occupied by vDSO/stack. > > I don't see the point. If you want to remap vDSO you can do that first, > and then reserve the full range. You don't need all that complexity. If we remap vDSO _without_ reserving memory first, then the kernel may end up choosing a reserved address for the new vDSO address, especially on 32-bit. We can implement the address allocation algorithm from scratch avoiding the preload area explicitly ourselves, but I suppose it would result in even more complexity. This is also why we reserve those ranges in the first place -- to let OS pick the address for us. > > And as general advice for your patches, please try to avoid changing > things that don't need changing, or adding infrastructure that isn't > needed. It will make it easier to see the actual changes. > My apologies for complicating the review work. I'll try to separate such patches into another serie, or omit them entirely if there's really no need for them. Perhaps there was something that I was thinking wrong about along the way. In that case, I'll try my best to explain my motivation and/or assumption behind those patches. I'd like to not repeat the same mistake again, so I'd be happy to learn about what went wrong (so that I can correct it). Please feel free to ignore them if you wish, though. - loader: Use long instead of int for syscall return type in i386 code. I was planning to add more syscalls, and found that i386 code used "int" for syscall returns. My assumption was that it was a style issue (presumably 32-bit-area Wine's leftover) that I was expected to address _before_ I could add more inline asm blocks. [The reason why I found "int" confusing was that I usually read "long" as "machine register-width integer," and "int" as "integer that either only needs to hold small values (e.g. exit code) _or_ has to be 32-bit for some reason." Granted int = long on ILP32, but someone looking at the code might not be able to immediately verify that the type is correct unless they also consider that "int $0x80" is for the i386 ABI (or that the code region is guarded by ifdef __i386__), and i386 is a 32-bit architecture. I supposed that would be extra cognitive load for someone reviewing/auditing the code.] Perhaps my mistake here was one of the following: 1. The code style wasn't an issue at all. I (incorrectly) assumed that the code style was a general consensus among systems programmers. 2. The code style was indeed a problem, but it wasn't really required to fix them in the *same* patch serie. 3. The style fix was required, but it came too early in the serie. - loader: Remove GCC <=4.x EBX register spilling workaround for i386. Similar to above, but debugging related. Again I was adding asm blocks around, so I felt it obliged to fix it before adding more code. - loader: Enable dumping additional vectors in dump_auxiliary. This is not a critical patch, but merely a debugging aid. Since it was obvious from the subject, I supposed it wouldn't hurt to include it towards the end of the series. I agree it would certainly have been better off as a separate patch, however. Meanwhile, following are patches that I deem necessary and not gratuitous: - loader: Refactor argv/envp/auxv management. This was necessary because I had to pass around the pointers to stack objects a lot. Examples include getenv() for letting user control remapping behaviour via env vars, and stack relocation (which requires shifting all the argv/envp/auxv pointers). If the pointer variables were not in one place, the latter would make the code a lot hairy (by passing around all the scattered stack pointers) and unmaintainable. - loader: Refactor number parsing to own function. Number parsing was only for WINEPRELOADRESERVE. Now I need it for parsing /proc/self/maps as well. I also looked through all of my past patchsets, and here's my thoughts about them: - kernel32/tests: Test module refcounting with forwarded exports. => You have pointed out that the IATGAS hack is ugly (which is true). Perhaps this one counts as an "unnecessary infrastructure." As for the justification of a new DLL (forward4), it was particularly hard to find a concrete example that exactly suits the tests' needs. 1. The DLL has to forward calls to another DLL *but* must not actually import from it. This excludes DLLs like kernel32 which both forwards to *and* imports from kernelbase. 2. The above condition must hold for all Windows versions being tested *and* in Wine. 3. The DLL must not have been already loaded (either directly or indirectly via dependencies) in the process. This excludes DLLs like kernel32, advapi, user32, gdi32, shell32, setupapi, userenv, and etc. The most recent iteration of this patchset uses ICMP/IPHLPAPI, but I'm still not 100% sure about the compatibility issue. An alternative would be to keep forward4 and modify winebuild so that TESTDLLs can import from other TESTDLLs, but it seemed like an overkill. - winedbg(gdbproxy): don't misbehave on module names w/ special characters; modernise => Patchset does contain a lot of refactoring work, but I tried to justify them in the commit message body to my best. - sock_recv() fix series => I attempted to minimize extra changes and/or infrastructure, but it might not have been perfect. In case some non-obvious change was needed, I tried to justify them in the patch message body. Perhaps there are other ways to implement them I couldn't think of. Meanwhile following patches are ostensibly gratuitous in nature (it might or might not be useful; it's up to your decision): - winedbg: use heap allocation for module filenames in handle_debug_event. => May seem gratuitous, but fixes module filenames being truncated over MAX_PATH. - server: Allow skipping debug handle retrieval in get_process_debug_info. => Functionality may be gratuitous, but may closely match Windows NT kernel more. Practially untestable (Wine lacks needed infrastructure) for now, so up to the maintainer's decision. - ntdll: Implement NtSetInformationVirtualMemory. => Functionality may be gratuitous or maybe some apps need it, up to the maintainer's decision. - ntdll: Implement __fastfail(). => Functionality may be gratuitous or maybe some apps need it, up to the maintainer's decision. - ntdll: Make syscall dispatcher properly restore X16 and X17 in ARM64. => Completely up to the maintainer's decision. - ws2_32/tests: Add order-agnostic check in test_simultaneous_async_recv. => Completely up to the maintainer's decision. Again, thanks a lot! -- Sincerely, Jinoh Kang

Jan. 26, 2022

Re: [PATCH 3/3] wined3d: Don't initialize system memory for buffers.

by Zebediah Figura (she/her)

On 1/26/22 07:47, Henri Verbeet wrote: > On Tue, 25 Jan 2022 at 03:35, Zebediah Figura <zfigura(a)codeweavers.com> wrote: >> This fixes test_map_synchronisation() on 64-bit machines, broken after >> 194b47b4fd92dda8ebf24e88ca7a14fc926c84ab. >> > How is it broken, and why does that happen? The test creates a buffer, maps it once, then maps it again with NOOVERWRITE while the GPU is still drawing, expecting the new data to be read by the GPU during the draw. On 32-bit machines, and 64-bit machines before 194b47b4fd9, we do the following: First map: uses SYSMEM since the BO is not created yet Draw: upload to VBO Second map: map the existing VBO with GL_MAP_UNSYNCHRONIZED_BIT After 194b47b4fd9, we don't use GL_MAP_UNSYNCHRONIZED_BIT since the buffer has READ access, which means that the second map will be synchronized and wait for the draw to complete. After this patch, we do the following: First map: create and map a VBO (not unsynchronized, but coherent and persistently mapped) Draw: use mapped VBO Second map: write to existing (coherent) VBO, which is unsynchronized I'll make sure to include this explanation in the patch next revision. Of course, although this motivated the patch, it seems like a good idea regardless. In most cases we don't want SYSMEM for buffer objects, especially for newer d3d versions, and we should avoid the overhead of allocating it and keeping it around. It may also be that we want to translate D3DVBCAPS_WRITEONLY into ~WINED3D_RESOURCE_ACCESS_MAP_R. My limited understanding is that this doesn't contradict test_vb_writeonly() per se, but rather would only be a bad idea performance-wise if ddraw applications rely on reading from such mapped buffers in critical paths. I don't know whether that's the case. > >> @@ -1233,7 +1233,7 @@ static HRESULT wined3d_buffer_init(struct wined3d_buffer *buffer, struct wined3d >> } >> buffer->buffer_ops = buffer_ops; >> buffer->structure_byte_stride = desc->structure_byte_stride; >> - buffer->locations = data ? WINED3D_LOCATION_DISCARDED : WINED3D_LOCATION_SYSMEM; >> + buffer->locations = WINED3D_LOCATION_DISCARDED; >> > I think avoiding WINED3D_LOCATION_SYSMEM would be great, but I'm not > as convinced about using WINED3D_LOCATION_DISCARDED for that purpose. > I'm not sure we've ever confirmed that buffer resources should be > zeroed after creation, but we have for texture resources; it would not > surprise me if this change would break some applications. > > My preferred way of handling this would be to introduce > WINED3D_LOCATION_CLEARED, which would effectively defer clearing the > resource until loading it into another location. When replacing the > entire (sub-)resource with e.g. a DISCARD map or sub-resource update, > we can then just skip that clear. This approach would also allow us to > avoid zeroing texture resources on creation, an in case of the Vulkan > renderer, we could even extend that a bit further and merge > render-target clears with starting render passes by using > VK_ATTACHMENT_LOAD_OP_CLEAR and appropriate clear values. I had in fact had that same idea :-) I left off using it for buffers because it wasn't clear to me that it was necessary. Thus far we had only found evidence that it was necessary for textures (and perhaps just swapchain textures?) Unfortunately I don't currently have access to any Windows machines (especially older machines), so I can't verify that buffers (or textures, for that matter) are implicitly cleared on initialization.

Jan. 26, 2022

Re: [PATCH] winevulkan: Enable VK_EXT_pipeline_creation_feedback.

by Henri Verbeet

On Wed, 26 Jan 2022 at 17:35, Georg Lehmann <dadschoorse(a)gmail.com> wrote: > > We did previously not support this extension because it runs into 32bit > alignment issues. But it is core in 1.3, so we already enabled the > functionality. Additionally vkd3d-proton wants to use the extension and it > does not care about 32bit. > Shouldn't that have blocked 1.3 support instead?

Jan. 26, 2022

[PATCH] winevulkan: Enable VK_EXT_pipeline_creation_feedback.

by Georg Lehmann

We did previously not support this extension because it runs into 32bit alignment issues. But it is core in 1.3, so we already enabled the functionality. Additionally vkd3d-proton wants to use the extension and it does not care about 32bit. Signed-off-by: Georg Lehmann <dadschoorse(a)gmail.com> --- dlls/winevulkan/make_vulkan | 1 - 1 file changed, 1 deletion(-) diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index c609bbfd151..6e09f018b84 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -96,7 +96,6 @@ UNSUPPORTED_EXTENSIONS = [ "VK_AMD_display_native_hdr", "VK_EXT_full_screen_exclusive", "VK_EXT_hdr_metadata", # Needs WSI work. - "VK_EXT_pipeline_creation_feedback", "VK_GOOGLE_display_timing", "VK_KHR_external_fence_win32", "VK_KHR_external_semaphore_win32", -- 2.35.0

Jan. 26, 2022

Re: [PATCH vkd3d v6 9/9] tests: Add tests for texture gather methods.

by Henri Verbeet

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>

Jan. 26, 2022

Re: [PATCH vkd3d v6 1/9] vkd3d-shader/hlsl: Introduce a sm4_src_from_node() helper.

by Henri Verbeet

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>

Jan. 26, 2022