March 2011 - Wine-devel - list.winehq.org

Re: wineport: Add fast builtin & asm versions of ffs() + ctz() where supported.
by Alexandre Julliard 11 Mar '11

11 Mar '11

Adam Martinson <amartinson(a)codeweavers.com> writes: > @@ -236,7 +241,40 @@ extern int getopt_long_only (int ___argc, char *const *___argv, > #endif /* HAVE_GETOPT_LONG */ > > #ifndef HAVE_FFS > -int ffs( int x ); > + #if defined(__i386__) || defined(__x86_64__) > + #define HAVE_FFS > + static inline int ffs( int x ) > + { > + if (!x) > + { > + return 0; > + } > + else > + { > + int ret; > + __asm__("bsfl %1, %0; incl %0" : "=r" (ret) : "r" (x)); > + return ret; > + } > + } > + #elif defined(__GNUC__) && GCC_VERSION >= 29503 > + #define HAVE_FFS > + #define ffs(x) __builtin_ffs(x) > + #else > + int ffs( int x ); > + #endif > +#endif You'd have to show benchmarks to prove that this complexity is necessary. Given that ffs() should already be inlined on all decent platforms, I doubt you'd be able to demonstrate a difference (if anything, your version would be slower because of the extra increment). -- Alexandre Julliard julliard(a)winehq.org

3 2

Re: wineport: Use a lookup table to make our slow ffs() a bit faster.
by Henri Verbeet 11 Mar '11

11 Mar '11

Something like the following may be faster: static const int bit_pos[] = { 0x00, 0x01, 0x0e, 0x02, 0x0f, 0x1b, 0x12, 0x03, 0x1e, 0x10, 0x1c, 0x16, 0x18, 0x13, 0x0a, 0x04, 0x1f, 0x0d, 0x1a, 0x11, 0x1d, 0x15, 0x17, 0x09, 0x0c, 0x19, 0x14, 0x08, 0x0b, 0x07, 0x06, 0x05, }; const unsigned int c = 0x7dc4d65; return bit_pos[((x & -x) * c) >> 27]; I'd expect that mostly due to eliminating the branches and the implicit shift & mask operations in something like "y.c[1]".

3 2

Re: wined3d/context: Remove a trivial call to context_validate_onscreen_formats() in context_apply_blit_state().
by Henri Verbeet 11 Mar '11

11 Mar '11

On 10 March 2011 20:22, Adam Martinson <amartinson(a)codeweavers.com> wrote: > - context_validate_onscreen_formats(device, context, NULL); > + /* It's silly to call context_validate_onscreen_formats() with a NULL depth_stencil. */ Sigh. I've asked you a very simple question about the performance impact of these changes *twice* now, and you ignored the question both times. Unless you have evidence that the compiler fails to eliminate the call by itself, and this actually makes any kind of difference, consistency is more important. The same goes for the IWineD3DDeviceImpl_MarkStateDirty() change, though at least in that case there's somewhat of a remote chance of it making a difference.

2 1

Re: include: Add rmxftmpl.h header.
by Alexandre Julliard 11 Mar '11

11 Mar '11

Dylan Smith <dylan.ah.smith(a)gmail.com> writes: > Manually parsed a hexdump of D3DRM_XTEMPLATES from the native header; > comments to show the corresponding text encoding. I used a test > application to dump D3DRM_XTEMPLATES with this header and the native > header to confirm that this header binary compatible with the native one. That stuff needs to be generated from source, you can't just copy the native header contents. -- Alexandre Julliard julliard(a)winehq.org

2 1

re: wineport: Use a lookup table to make our slow ffs() a bit faster.
by Dan Kegel 11 Mar '11

11 Mar '11

Do you actually have a system that doesn't have HAVE_FFS set? On my machine, int main(int argc, char **argv) { return ffs(argc); } compiles to pushl %ebp movl %esp, %ebp movl $-1, %edx bsfl 8(%ebp), %eax cmove %edx, %eax addl $1, %eax popl %ebp ret which is probably pretty fast. Large lookup tables can dirty the L1 cache and slow other things down. See also http://stackoverflow.com/questions/757059/position-of-least-significant-bit…

2 1

Re: [PATCH 1/6] user32: Add get/release routines for accessing cursor frames.
by Alexandre Julliard 11 Mar '11

11 Mar '11

Erich Hoover <ehoover(a)mines.edu> writes: > @@ -135,6 +139,16 @@ static void release_icon_ptr( HICON handle, struct cursoricon_object *ptr ) > release_user_handle_ptr( ptr ); > } > > +struct static_cursoricon_object *get_icon_data( struct cursoricon_object *obj, int istep ) > +{ > + return &obj->frames[istep]; > +} > + > +void release_icon_data( struct cursoricon_object *obj, int istep, struct static_cursoricon_object *icon_data ) > +{ > + /* placeholder */ > +} I find your data structures confusing. You add a concept of icon_data separate from icon_ptr, but it doesn't correspond to the data structures, there's no such thing as an icon_data. There should be a way of making this clearer. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: programs/dxdiag: beginnings of support for /t option
by Henri Verbeet 11 Mar '11

11 Mar '11

On 10 March 2011 20:35, Drew Goodwin <drew(a)drewgoodwin.com> wrote: > + length = MultiByteToWideChar(0, 0, name, -1, NULL, 0)+1; > + { > + /* Allocate on the stack */ > + WCHAR nameW[length]; Oh, and variable length arrays are a C99 feature, Wine tries to mostly stick to C89.

2 1

New winetricks 20110311b: winetricks is dead, long live winetricks
by Dan Kegel 11 Mar '11

11 Mar '11

winetricks-20110311b is now released. Changes since winetricks-20110123 (for anyone who has not been using the alpha versions): - manages multiple wineprefixes - apps are now installed into separate wineprefixes by default - gui now split into separate pages for games, benchmarks, apps, dlls, fonts - gui now shows approximate disk requirement and install times - gui now lets you pick silent installation mode - apps that are broken (e.g. have drm that wine can't handle) are now hidden by default, gui has a button to reveal them - 70 games added, including Dragon Age II demo, Starcraft II demo, Mass Effect 2, and Mass Effect 2 demo - many updates and bugfixes Changes since winetricks-alpha-20110310: - mdac25, mdac27 removed, as downloads no longer available - mdac28: updated to sp1 - sizes/times in gui updated. (Most show download + install time; a future release will show just install time.) For bug tracker, mailing lists, and wiki, see http://winetricks.org Download url: http://winetricks.org/winetricks

1 0

re: links down
by Dan Kegel 11 Mar '11

11 Mar '11

Looks like 2.8 sp1 is still up, though: http://www.microsoft.com/downloads/en/details.aspx?familyid=78cac895-efc2-4… I'll update winetricks, thanks. (And I'll put together a fast link checker for winetricks; doing a full run with an empty cache takes too long to be a good way of noticing broken links.) Wish I'd noticed this sooner!

1 0

Re: programs/dxdiag: beginnings of support for /t option
by Henri Verbeet 11 Mar '11

11 Mar '11

On 10 March 2011 20:35, Drew Goodwin <drew(a)drewgoodwin.com> wrote: > I'm a CS student at UCLA. I and some classmates are adding features to > wine's dxdiag. Here's the first patch, with code by Steve Feng, Andrew > Freer, and myself, properly copyrighted and licensed under the LGPL. Once > this is accepted, we will send a series of patches to support the remaining > sections in the /t option and the /x option, and add a working gui. > Just some random comments, this is by no means a proper review: > -EXTRADEFS = -DWINE_NO_UNICODE_MACROS > +EXTRADEFS = -DWINE_NO_UNICODE_MACROS -D__WINE__ What do you need __WINE__ for? > +IMPORTS = advapi32 comdlg32 comctl32 d3d9 d3dx9 ddraw dsound dxguid gdi32 kernel32 ole32 oleaut32 shell32 user32 winmm > + That's a lot of imports, do you really need all of those in this patch? > +HINSTANCE g_hinstance; The way you're using this, that's just a slightly odd way to write "NULL". > + sysinfo = malloc(sizeof(DxDiag_SystemInfo)); You should typically use HeapAlloc() for memory allocation in Wine, except for rare cases where you pass the result to a library function that free()'s it. You also leak the sysinfo, but perhaps that's ok as long as you exit right after being done with it. The "g_dxdiagcom_root" global isn't very useful, just pass it to OutputPlainFile() directly.

1 0