Support for Windows TLS API #1

Merged
merged 2 commits into from Aug 8, 2012

3 participants

@malkia

Hi regal team,

The Windows XP with MSVCRT.DLL is my min-spec target, and I'm using the Windows WDK 7.1 to compile it.

Compiled this way, it runs fine under Vista / Windows 7, but it does not on XP. The problem is that __declspec(thread) is not supported there with the combination above.

Given that REGAL uses __declspec(thread) in isolated way, I've mimicked the pthread approach, and wrote a version, disabled by default, and enabled with #define REGAL_WIN_TLS 1 that uses the Widnows TLS API - TlsAlloc, TlsSet/GetValue, etc.

I've tested that under the one machine that I have with Windows XP Service Pack 3 (32-bit) and works so far.

Thanks,
Dimiter 'malkia' Stanev

@malkia malkia Added REGAL_WIN_TLS #define - REGAL would use the Windows TLS API (Tl…
…sAlloc, TlsSet/GetValue, etc.) instead of __declspec(thread). This allows REGAL to be compiled with Windows WDK 7.1 targeting MSVCRT.DLL and working on Windows XP Service Pack 3 (tested)
38b2a23
@nigels-com nigels-com was assigned Aug 8, 2012
@nigels-com

Looks good to me. I'll do some homework first...

@malkia

Thanks Nigel!

It seems that __declspec(thread) won't work with dynamic libraries that use it, but are loaded after the first (well second excluding the main one) thread is created - Check Raymond Chen's blog - http://blogs.msdn.com/b/oldnewthing/archive/2010/11/22/10094489.aspx

EDIT: If a DLL is loaded with LoadLibrary() and has __declspec(thread) it won't work. But thas has been fixed on Vista and above. I guess that's why I was having problems with XP.

On the downside of using TLS api - is that it's somewhat slower (probably), instead of directly using the fs:[_tls_index] to access the register, it goes through couple of jumps to do it (but then again, anyone thinking that speeding up OpenGL calls is the silver bullet, is asking for misery).

@casseveritt
p3 member

Malkia,

We're leaning toward using the TLS api only for Windows to keep the code simpler and more portable.

Thanks -
Cass

@nigels-com nigels-com merged commit b157e6a into p3:master Aug 8, 2012
@nigels-com

It's merged in, thanks for the contribution.
I think we may switch to this path being the default, at least.

  • Nigel
@malkia
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment