[Lldb-commits] [PATCH] Make CMake choose the test exe target architecture according to the build.

[Reid Kleckner]

Seems fine to me, but I'll let Chandler respond.

================
Comment at: CMakeLists.txt:22-26
@@ -21,1 +21,7 @@
 
+# Determine the native architecture.
+string(TOLOWER "${LLVM_TARGET_ARCH}" LLDB_TARGET_ARCH)
+if( LLDB_TARGET_ARCH STREQUAL "host" )
+  string(REGEX MATCH "^[^-]*" LLDB_TARGET_ARCH ${LLVM_HOST_TRIPLE})
+endif ()
+
----------------
Zachary Turner wrote:
> Chandler Carruth wrote:
> > So, this isn't the "native" architecture. For an LLVM cross compiler, it will not execute on the local machine at all. It's just the target architecture.
> > 
> > I don't think you want to have an 'LLDB_TARGET_ARCH' variable. That doesn't really make sense, especially in the singular.
> > 
> > I think you should just set this in the test-specific cmake, and I think you should make it 'LLDB_TEST_TRIPLE' which is cached, user-overridable, and defaults to 'LLVM_DEFAULT_TARGET_TRIPLE'.
> LLDB doesn't support cross-compilation anyway, so I think it's safe to assume that the host and target are the same.
> 
> All we're trying to figure out, ultimately, is whether to pass -m32 or -m64 to clang.  So allowing the user to specify a full triple seems like overkill and is likely to be confusing, it seems more straightforward if they just pass in "x86" or "x64".
Just doing the architecture seems reasonable for now. The triple could be useful if we ever want to support running the test suite in a way that is remotely debugging binaries in a VM with a different OS or something. That's probably either not useful or a long way off.

================
Comment at: test/CMakeLists.txt:24
@@ -29,1 +23,3 @@
+
+STRING(TOLOWER "${LLDB_TEST_ARCH}" LLDB_TEST_ARCH)
 
----------------
nit: s/STRING/string/, we appear to prefer lower-case cmake function names.

http://reviews.llvm.org/D4730