[Parallel_libs-commits] [PATCH] D22687: [StreamExecutor] Add utility library

[Justin Lebar via Parallel_libs-commits]

jlebar added a comment.

I'll look over the changes to error.h after I get some food (and finally make my way into the office), but here are a few quick things.


================
Comment at: streamexecutor/CMakeLists.txt:50
@@ +49,2 @@
+
+add_subdirectory(lib)
----------------
FWIW I have studiously avoided learning CMake.  If this isn't just copy/pasted from somewhere, maybe Art can sign off on it.  But if you're happy with it, that's good enough for me.

================
Comment at: streamexecutor/include/streamexecutor/Utils/ThreadAnnotations.h:28
@@ +27,3 @@
+/// held when accessing the annotated variable.
+#define SE_GUARDED_BY(x) SE_THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
+
----------------
jhen wrote:
> jlebar wrote:
> > Would rather not pollute the global namespace with SE_THREAD_ANNOTATION_ATTRIBUTE if we can help it.  (That is, would rather declare this with one macro rather than two.)
> > 
> > But this also seems like something that would be better in e.g. mutex.h.
> Consolidated to one macro and moved to Mutex.h.
Ah, sorry, I meant, it seems that this belongs in LLVM's mutex.h (as a separate patch).


https://reviews.llvm.org/D22687