proposed fix for gemm thread safety; using thread-local storage

[guacamoleo]

each thread will have its own kernel map and, therefore, its own set of kernels

[kknox]

Reviewed 1 of 1 files at r1. Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/library/blas/xgemm.cc, line 152 [r1] (raw file): This is a redundant #include; duplicate from top of file

---

src/library/blas/xgemm.cc, line 156 [r1] (raw file): Is there a reason you did not declare and initialize kernel_map in the same statement?

---

Comments from the review on Reviewable.io

[pavanky]

Hmm. This particular one was because of me. Sorry about that. Is this the only cause of concern for thread safety ? I thought I saw some global variables in the code that's been auto-generated that might cause some issues.

[guacamoleo]

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

---

_src/library/blas/xgemm.cc, line 152 [r1] (raw file):_ fixed

---

src/library/blas/xgemm.cc, line 156 [r1] (raw file): this is a requirement/limitation of thread local storage. Not using a pointer and calling the constructor of kernel_map() isn't allowed. It may be allowed in c++11.

---

Comments from the review on Reviewable.io

[kknox]

@pavanky I'm curious, can you see the 'reviewable' code review? Do you have permissions to comment and review?

[guacamoleo]

I also tested test-functional THREAD and it passes too.

[pavanky]

FYI C++11 does have std::thread_local that allows you to use constructors. The issue however is the availability of this feature. Visual Studio only got it with VS2015. AppleClang does not implement the feature yet.

GCC and Clang have had it for a while now.

[guacamoleo]

Yes, we plan to use such c++11 features in future libraries; but for the current clBLAS, we’re trying to stay compatible with older versions.

David E. Tanner

---

Sr Software Engineer | Radeon Technologies Group – Open Compute

From: Pavan Yalamanchili [mailto:notifications@github.com] Sent: Thursday, March 03, 2016 6:03 PM To: clMathLibraries/clBLAS Cc: Tanner, David Subject: Re: [clBLAS] proposed fix for gemm thread safety; using thread-local storage (#235)

FYI C++11 does have std::thread_local that allows you to use constructors. The issues however is the availability of this feature. Visual Studio only got it with VS2015. AppleClang does not implement the feature yet.

— Reply to this email directly or view it on GitHubhttps://github.com/clMathLibraries/clBLAS/pull/235#issuecomment-192028642.

[kknox]

Reviewed 1 of 1 files at r3. Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io