[PATCH] D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler.

[Emily Shi via Phabricator via llvm-commits]

aralisza added inline comments.


================
Comment at: compiler-rt/test/CMakeLists.txt:5
+option(COMPILER_RT_TEST_BUILT_LIBS
+  "When testing use the just built runtime libraries instead of the compiler's. \
+  This option has no effect when the compiler-rt build library directory is  \
----------------
nit, is there a missing comma here? I had to read this a few times to parse it lol

```
When testing, use the just built runtime libraries instead of the compiler's.
```


================
Comment at: compiler-rt/test/lit.common.cfg.py:30
+  if config.compiler_id != 'Clang':
+    lit_config.warning('Failed to determine compiler\'s runtime directory')
+    # TODO: Support other compilers.
----------------
do you think it'll be useful to include that the compiler is not supported yet here?


================
Comment at: compiler-rt/test/lit.common.cfg.py:130
+      'Compiler lib dir != compiler-rt lib dir\n'
+      f'Compiler libdir: {compiler_libdir}\n'
+      f'compiler-rt lib dir: {compiler_rt_libdir_real}')
----------------
super nitpicky, but can you make it so the paths align to make it easier to compare? like
```
f'Compiler libdir:       {compiler_libdir}\n'
f'compiler-rt libdir:    {compiler_rt_libdir_real}')
```


================
Comment at: compiler-rt/test/lit.common.cfg.py:136-143
+        lit_config.fatal(
+            'The compiler being used for testing is not using runtime '
+            'libraries created by this build but testing is configured to test '
+            'the built libraries (COMPILER_RT_TEST_BUILT_LIBS is ON). '
+            'Testing cannot proceed. To fix this modify this test suite to support this testing configuration.'
+            'Alternatively set COMPILER_RT_TEST_BUILT_LIBS to OFF to test the '
+            'runtime libraries of the compiler instead.'
----------------
I'm having a hard time understanding this message, does it mean something like

> COMPILER_RT_TEST_BUILT_LIBS=ON, but this test suite does not support running using the just-built runtime libraries.
> Either modify this test suite to support this test configuration, or set COMPILER_RT_TEST_BUILT_LIBS=OFF to test the runtime libraries included in the compiler instead.


================
Comment at: compiler-rt/test/lit.common.cfg.py:147
+      config.compiler_rt_libdir = compiler_libdir
+    lit_config.warning(f'Testing using libraries in {config.compiler_rt_libdir}')
+
----------------
should this be a warning? It seems like info to me


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101681/new/

https://reviews.llvm.org/D101681