[PATCH] D12001: Implement __emutls_get_address

[Chih-Hung Hsieh via llvm-commits]

chh added a comment.

PTAL new diff 7. Thanks.


================
Comment at: compiler-rt/lib/builtins/CMakeLists.txt:44
@@ -43,2 +43,3 @@
   divxc3.c
+  emutls.c
   enable_execute_stack.c
----------------
danalbert wrote:
> compnerd wrote:
> > Please don't add this to GENERIC_SOURCES.  This is really not applicable to all targets.  It uses pthread, so it can't be used on say Windows.
> > 
> > This makes me wonder if this actually belongs in the compiler builtins in the first place.
> GCC keeps it in libgcc, and it's a function that's only ever emitted by the compiler as a replacement for `__thread`. Seems like this is the right place to me. A windows equivalent could be implemented as well, but perhaps it's best to just wrap all this in an `#if !defined(_WIN32)` for now.
I will exclude it from WIN32 as danalbert suggested.


================
Comment at: compiler-rt/lib/builtins/emutls.c:80
@@ +79,3 @@
+    /* Make sure that align is power of 2. */
+    if ((((align - 1) | align) + 1) != (align << 1))
+        abort();
----------------
compnerd wrote:
> Cant this be written more simply as
> 
>     (align & (align - 1) == 0)
Done, use if ((align & (align - 1)) != 0) abort();


================
Comment at: compiler-rt/lib/builtins/emutls.c:109
@@ +108,3 @@
+        }
+    }
+    free(ptr);
----------------
danalbert wrote:
> compnerd wrote:
> > Remove the unnecessary braces here.
> Bodies that aren't a one-liner should have braces.
I think you meant that line 106 does not need brace and line 105 needs brace. Fixed it.




http://reviews.llvm.org/D12001