[PATCH] D29030: [LNT] Add cross-compilation support to 'lnt runtest test-suite'

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: lnt/tests/test_suite.py:760-763
+        # FIXME: this probably needs to be conditionalized on the compiler
+        # being clang
+        if "c_target" in cmake_vars:
+            cflags += " --target=" + cmake_vars["c_target"]
----------------
Shouldn't we rather put the `--target` flag into the toolchain file, is that not possible?

If we need to do it from the lnt side anyway, I'd recommend setting `TEST_SUITE_ARCH_FLAGS` which is a convention I introduced to the CMakeLists.txt. These are inserted to all compiler and linker invocations (so do not depend on our current "misfeature" that cflags are passed to C compiler, C++ compiler and linker) and plays nicer with people who override the CFLAGS in cache files.


================
Comment at: lnt/tests/test_suite.py:766
 
-    def _get_cc_info(self):
+    def _extract_cmake_vars_from_cache(self):
         assert self.configured is True
----------------
This name feels wrong to me as this does not return the cmake variables which would be `CMAKE_C_COMPILER`, `CMAKE_BUILD_TYPE`, ... but instead returns some LNT specific names like 'cc', 'build_type', ...


================
Comment at: lnt/tests/test_suite.py:797
+    def _get_cc_info(self):
+        cmake_vars = self._extract_cmake_vars_from_cache()
         return lnt.testing.util.compilers.get_cc_info(
----------------
Looking at the earlier usage of these functions, `_extract_cmake_vars_from_cache()` is called once anyway while possible `_get_cc_info()` calls repeat this, maybe you better let the user feed in the `cmake_vars` information as an argument since it is available anyway.


https://reviews.llvm.org/D29030