[test-suite] Added llvm_target_prefix() command to set unique target prefix.

[tra]

Summary

Instead of relying on magic to generate unique names, allow user to specify unique prefix for target names.
llvm_target_prefix() sets the prefix for targets created with llvm_singlesource()/llvm_multisource().

[MatzeB]

While the patch looks good to me, what about the alternative approach of just error out on a name clash? AFAIK the only clash we have is between SingleSource/Benchmarks/Shootout and SingleSource/Benchmarks/Shootout-C++ and I'd rather go for a low-tech solution like allowing to specify a prefix in the shootout CMakeFiles and throw an error on non-unique targets instead of going through all the trouble to automatically create unique names.

[tra]

How about adding llvm_test_name_prefix() which would add given prefix to all targets generated within scope?
Default will remain empty, but for testsuites that have name clashes it would provide an easy way to make their names unique.
Or we can even require having prefix set before we call llvm_multisource(). With that in place we can remove unique name magic and error out on name clashes.

[MatzeB]

In D19423#409448, @tra wrote:

How about adding llvm_test_name_prefix() which would add given prefix to all targets generated within scope?
Default will remain empty, but for testsuites that have name clashes it would provide an easy way to make their names unique.
Or we can even require having prefix set before we call llvm_multisource(). With that in place we can remove unique name magic and error out on name clashes.

Yep that's what I had in mind. Though I think using a variable like TARGET_PREFIX instead of a function would be more in-line with the current way the singlesource()/multisource() are configured.

[MatzeB]

cmake/modules/SingleMultiSource.cmake

17–24 ↗

(On Diff #54716)

Is there a benefit in having a function here instead of just letting users set TARGET_PREFIX immediately?

26–47 ↗

(On Diff #54716)

I just tried it and cmake errors out by itself if you create two (logical) targets with the same name if you set:
cmake_policy(SET CMP0002 NEW)

[tra]

cmake/modules/SingleMultiSource.cmake

17–24 ↗

(On Diff #54716)

None, other than separating end-user from the details of implementation.
Similarly to llvm_test_run()/llvm_test_verify() it does *something* (that user does not need to be aware of) to make llvm_multisource() do the right thing.

26–47 ↗

(On Diff #54716)

It does error out, but having explicit "please set prefix" message may make experience of adding new tests somewhat less painful.

[MatzeB]

LGTM

test-suite/trunk/SingleSource/Benchmarks/Shootout-C++/CMakeLists.txt

+llvm_target_prefix("shootout-cxx")
 list(APPEND CXXFLAGS -Wno-deprecated)
 list(APPEND CPPFLAGS -Wno-deprecated)
 set(FP_TOLERANCE 0.00000001)
 if(ARCH STREQUAL "XCore")
   if(NOT DEFINED SMALL_PROBLEM_SIZE)
     set(XCORE_TARGET_NEEDS_MEMORY 256)
     set(RUNTIMELIMIT 10800)
   endif()

test-suite/trunk/SingleSource/Benchmarks/Shootout/CMakeLists.txt

+llvm_target_prefix("shootout")
 list(APPEND LDFLAGS -lm)
 if(ARCH STREQUAL "XCore")
   set(XCORE_TARGET_NEEDS_MEMORY 256)
 endif()
 if(TEST_SUITE_BENCHMARKING_ONLY)
   list(APPEND PROGRAMS_TO_SKIP hello)
 endif()
 llvm_singlesource()

test-suite/trunk/SingleSource/Regression/C++/CMakeLists.txt

+llvm_target_prefix("regression-cxx")
 list(APPEND LDFLAGS -lstdc++)
 llvm_singlesource()
 
 llvm_add_subdirectories(EH)

test-suite/trunk/SingleSource/Regression/C/CMakeLists.txt

+llvm_target_prefix("regression-c")
 if(ARCH STREQUAL "x86")
   if(DEFINED USE_REFERENCE_OUTPUT)
     set(EXEC_XFAILS casts)
   endif()
 endif()
 list(APPEND LDFLAGS -lm)
 llvm_singlesource()

test-suite/trunk/cmake/modules/SingleMultiSource.cmake

 ##===- SingleMultiSource.cmake --------------------------------------------===##
 #
 # Defines helpers to add executables and tests. The entry points to this
 # file are:
 #   `llvm_singlesource()` and
 #   `llvm_multisource()`
 #
 # Each is a macro that uses the environment it was called in to determine
 # what to build and how, and generates a test file that can be given to LIT.
 # The test file is generated at configure time.
 #
 ##===----------------------------------------------------------------------===##
 
 include(TestFile)
 
+
+# Set unique target prefix within caller's scope.
+function(llvm_target_prefix prefix)
+  if(prefix)
+    set(TARGET_PREFIX "${prefix}-" PARENT_SCOPE)
+  else()
+    set(TARGET_PREFIX "" PARENT_SCOPE)
+  endif()
+endfunction()
+
 # Given a source file name after which a test should be named, create a unique
 # name for the test. Usually this is just the source file with the suffix
-# stripped, but in some cases this ends up causing duplicates so attempt to
-# make each unique (by adding pathname segments until they become unique).
-#
-# FIXME: Swap this with a simpler procedure to just append a numeral
+# stripped, and ${TARGET_PREFIX} prepended.
 set_property(GLOBAL PROPERTY registered_executables)
 function(get_unique_exe_name new_name main_src)
   get_property(registered_executables GLOBAL PROPERTY registered_executables)
 
   string(REGEX REPLACE ".[cp]+$" "" path ${main_src})
-  string(REGEX REPLACE ".*/" "" name ${path})
+  get_filename_component(name ${path} NAME )
+  set(name "${TARGET_PREFIX}${name}")
+
   list(FIND registered_executables ${name} name_idx)
 
   if(${name_idx} EQUAL -1)
     set(${new_name} ${name} PARENT_SCOPE)
     set_property(GLOBAL APPEND PROPERTY registered_executables ${name})
     return()
   endif()
 
-  # There is a clash. Rename the target. Each time around the loop pull in
-  # a new path component.
-  foreach(n RANGE 1 4)
-    string(REGEX REPLACE ".*/([^/]+/${name})" "\\1" name ${path})
-    string(REGEX REPLACE "/" "-" safe_name ${name})
-
-    list(FIND registered_executables ${safe_name} name_idx)
-    if(${name_idx} EQUAL -1)
-      set(${new_name} ${safe_name} PARENT_SCOPE)
-      set_property(GLOBAL APPEND PROPERTY registered_executables ${safe_name})
-      return()
-    endif()
-  endforeach()
-  message(FATAL_ERROR "Failed to uniquify executable name!")
+  message(FATAL_ERROR "Duplicate executable name!"
+    "Please set unique prefix with llvm_target_prefix().")
 endfunction()
 
 # Add flags to a cmake target property.
 macro(append_target_flags propertyname target)
   if(NOT "${ARGN}" STREQUAL "")
     get_target_property(old_flags ${target} ${propertyname})
     if(${old_flags} STREQUAL "old_flags-NOTFOUND")
       set(old_flags)