[Parallel_libs-commits] [PATCH] D24302: Add streamexecutor-config

[Jason Henline via Parallel_libs-commits]

jhen added inline comments.

================
Comment at: streamexecutor/tools/streamexecutor-config/streamexecutor-config.in:129
@@ +128,3 @@
+      raise
+    print('llvm-config not found in PATH, please add it to the PATH and retry')
+    return
----------------
jlebar wrote:
> Nit, write to stderr (`print('foo', file=sys.stderr)`)?  (Actually not quite a nit because of how this program is going to be used -- ideally we wouldn't print anything other than flags to stdout.)
Since I'm using sys.exit here now, I'm just passing the error message to sys.exit instead and that will print it to stderr as well as returning error code 1.

================
Comment at: streamexecutor/tools/streamexecutor-config/streamexecutor-config.in:161
@@ +160,3 @@
+
+  print(' '.join(all_flags))
+
----------------
jlebar wrote:
> Should we do something to tokenize the flags?  (So that e.g. `-Idir with spaces` shows up correctly?)
Great catch! I added explicit quotes around @CMAKE_INSTALL_PREFIX@ everywhere it is used. This made it slightly more complicated to check whether a token being added by SE was already present in the LLVM output, so I added a new function `has_token` to do the token escaping before doing the search.

As for the output of llvm-config, I'll just trust that llvm-config quotes things correctly.


https://reviews.llvm.org/D24302