Re: Gallium: Fix for tgsi_emit_sse2()
So, we should really fix this. The two options are : - Keep a different calling convention under linux (cdecl by default, which requires saving esi by hand in the shader) and apply Victor's patch which saves restores this register - Use the same calling convention on all platforms, that is change include/pipe/p_compiler.h to define XSTDCALL to stdcall on linux, because for now it's empty, which is _not_ stdcall but cdecl. In any case, this is a serious issue as under linux esi gets corrupted on return from the SSE call. Which, of course, causes crashes. Stephane - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Gallium: Fix for tgsi_emit_sse2()
Sorry, this slipped through the net a little... Given how much is hardcoded with rtasm, I'd prefer to use a single calling convention everywhere, whether that's STDCALL or CDECL or something else I don't mind. Probably STDCALL because some compilers are too dumb to use anything else?? In which case, a little comment documentating what stdcall really means would help a lot - I've hit similar issues with the differences between calling conventions and it basically boiled down to disassembling gcc's output to figure out what we're supposed to be doing... If we switch to stdcall, there are a couple of other platform-specific varients in the generated code that can be removed. It's probably going to be the cleanest solution from the point of view of actually working on this code. Keith - Original Message From: Stephane Marchesin [EMAIL PROTECTED] To: Victor Stinner [EMAIL PROTECTED] Cc: dri-devel@lists.sourceforge.net Sent: Wednesday, April 2, 2008 12:18:33 PM Subject: Re: Gallium: Fix for tgsi_emit_sse2() So, we should really fix this. The two options are : - Keep a different calling convention under linux (cdecl by default, which requires saving esi by hand in the shader) and apply Victor's patch which saves restores this register - Use the same calling convention on all platforms, that is change include/pipe/p_compiler.h to define XSTDCALL to stdcall on linux, because for now it's empty, which is _not_ stdcall but cdecl. In any case, this is a serious issue as under linux esi gets corrupted on return from the SSE call. Which, of course, causes crashes. Stephane - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Gallium: Fix for tgsi_emit_sse2()
Hi, Here is my first patch for Nouveau project: a fix for 3D software rendering using SSE2 instruction set. The problem is that Gallium doesn't save/restore used registers (eax, edx, ecx, esi in my case). So I added push/pop in tgsi_emit_sse2(). My patch is quick and dirty, eg. tgsi_emit_sse2_fs() is not fixed. I don't know Gallium enough to write better fix ;-) Victor Stinner aka haypo diff --git a/src/gallium/auxiliary/draw/draw_vs_sse.c b/src/gallium/auxiliary/draw/draw_vs_sse.c index a4503c1..f11f9c6 100644 diff --git a/src/gallium/auxiliary/tgsi/exec/tgsi_sse2.c b/src/gallium/auxiliary/tgsi/exec/tgsi_sse2.c index 4e80597..b0bf49a 100755 --- a/src/gallium/auxiliary/tgsi/exec/tgsi_sse2.c +++ b/src/gallium/auxiliary/tgsi/exec/tgsi_sse2.c @@ -1998,6 +1998,18 @@ emit_instruction( case TGSI_OPCODE_RET: case TGSI_OPCODE_END: + emit_pop( + func, + get_temp_base() ); + emit_pop( + func, + get_const_base() ); + emit_pop( + func, + get_output_base() ); + emit_pop( + func, + get_input_base() ); #ifdef WIN32 emit_retw( func, 16 ); #else @@ -2248,22 +2260,35 @@ tgsi_emit_sse2( func-csr = func-store; + emit_push( + func, + get_input_base() ); + emit_push( + func, + get_output_base() ); + emit_push( + func, + get_const_base() ); + emit_push( + func, + get_temp_base() ); + emit_mov( func, get_input_base(), - get_argument( 0 ) ); + get_argument( 0+4 ) ); emit_mov( func, get_output_base(), - get_argument( 1 ) ); + get_argument( 1+4 ) ); emit_mov( func, get_const_base(), - get_argument( 2 ) ); + get_argument( 2+4 ) ); emit_mov( func, get_temp_base(), - get_argument( 3 ) ); + get_argument( 3+4 ) ); tgsi_parse_init( parse, tokens ); - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace-- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Gallium: Fix for tgsi_emit_sse2()
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Victor Stinner wrote: | Here is my first patch for Nouveau project: a fix for 3D software | rendering using SSE2 instruction set. | | The problem is that Gallium doesn't save/restore used registers (eax, | edx, ecx, esi in my case). So I added push/pop in tgsi_emit_sse2(). Doesn't the ABI say that those are scratch registers? That is, they don't need to be saved by the callee. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) iD8DBQFH69mVX1gOwKyEAw8RAjkFAJkBC8L/pRsrKjIcb4/YF4qQovm6qACghN6U VTvfpZR6FzWCxTEPuRp7sBs= =44VK -END PGP SIGNATURE- - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: Gallium: Fix for tgsi_emit_sse2()
Here is my first patch for Nouveau project: a fix for 3D software rendering using SSE2 instruction set. The problem is that Gallium doesn't save/restore used registers (eax, edx, ecx, esi in my case). So I added push/pop in tgsi_emit_sse2(). Doesn't the ABI say that those are scratch registers? That is, they don't need to be saved by the callee. On Linux, the calling method is cdecl. Marcheu told me that EAX, ECX and EDX can be used but not ESI (ESI should be saved). So here is a smaller patch: only save/restore ESI (temp base). About the crash: it occurs with Nouveau driver (yesterday GIT version) and NeverBall game. The bug only occurs with gcc 4.2, not with gcc 4.1. Gallium (mesa) is compiled with -O (-O1). Victor Stinner PS: I just subscribed to the mailing list diff --git a/src/gallium/auxiliary/draw/draw_vs_sse.c b/src/gallium/auxiliary/draw/draw_vs_sse.c index a4503c1..f11f9c6 100644 diff --git a/src/gallium/auxiliary/tgsi/exec/tgsi_sse2.c b/src/gallium/auxiliary/tgsi/exec/tgsi_sse2.c index 4e80597..2d4e707 100755 --- a/src/gallium/auxiliary/tgsi/exec/tgsi_sse2.c +++ b/src/gallium/auxiliary/tgsi/exec/tgsi_sse2.c @@ -1998,6 +1998,9 @@ emit_instruction( case TGSI_OPCODE_RET: case TGSI_OPCODE_END: + emit_pop( + func, + get_temp_base() ); #ifdef WIN32 emit_retw( func, 16 ); #else @@ -2248,22 +2251,26 @@ tgsi_emit_sse2( func-csr = func-store; + emit_push( + func, + get_temp_base() ); + emit_mov( func, get_input_base(), - get_argument( 0 ) ); + get_argument( 0+1 ) ); emit_mov( func, get_output_base(), - get_argument( 1 ) ); + get_argument( 1+1 ) ); emit_mov( func, get_const_base(), - get_argument( 2 ) ); + get_argument( 2+1 ) ); emit_mov( func, get_temp_base(), - get_argument( 3 ) ); + get_argument( 3+1 ) ); tgsi_parse_init( parse, tokens ); @@ -2327,22 +2334,26 @@ tgsi_emit_sse2_fs( func-csr = func-store; /* DECLARATION phase, do not load output argument. */ + emit_push( + func, + get_temp_base() ); + emit_mov( func, get_input_base(), - get_argument( 0 ) ); + get_argument( 0+1 ) ); emit_mov( func, get_const_base(), - get_argument( 2 ) ); + get_argument( 2+1 ) ); emit_mov( func, get_temp_base(), - get_argument( 3 ) ); + get_argument( 3+1 ) ); emit_mov( func, get_coef_base(), - get_argument( 4 ) ); + get_argument( 4+1 ) ); tgsi_parse_init( parse, tokens ); - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace-- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
