[Builtins] Provide a mechanism to selectively disable tests based on whether an implementation is provided by a builtin library.

[delcypher]

Summary

If a platform removes some builtin implementations (e.g. via the
Darwin-excludes mechanism) then this can lead to test failures because
the test expects an implementation to be available.

To solve this lit features are added for each configuration based
on which sources are included in the builtin library. The features
are of the form librt_has_<name> where <name> is the name of the
source file with the file extension removed. This handles C and
assembly sources.

With the lit features in place it is possible to make certain tests
require them.

Example:

REQUIRES: librt_has_comparedf2

All top-level tests in test/builtins/Unit (i.e. not under
arm, ppc, and riscv) have been annotated with the appropriate
REQUIRES: librt_has_* statement.

rdar://problem/55520987

[delcypher]

Add sanity checks to lit config.

[delcypher]

@beanz @steven_wu @arphaman @phosek @dexonsmith @thakis Ping.

[phosek]

LGTM % a few nits

test/builtins/Unit/lit.cfg.py

101

No need to check the length, if builtins_source_features: is sufficient.

108

I'd slightly prefer to do:

if builtin_source_feature not in builtins_source_features_set:
  builtins_source_features_set.add(builtin_source_feature)
else:
  builtins_source_feature_duplicates.append(builtin_source_feature)

It seems easier to comprehend.

[delcypher]

test/builtins/Unit/lit.cfg.py

108

Agreed. That is easier to read. The original code I wrote was written that way is to avoid doing two set look ups, instead the original code only does one.

However this is probably a very premature optimization at the expense of readability so let's go with the more readable option for now.

[delcypher]

• Rename crt_has_* -> librt_has_* so consistency with %librt lit substitution.
• Annotate as many tests as possible.
• Make python fixups suggested by Petr Hosek.

[delcypher]

@beanz @steven_wu @arphaman @phosek @dexonsmith @thakis I've improved this patch by annotating as many tests as possible. Okay to land?

[delcypher]

Ping.

[arphaman]

LGTM

[delcypher]

Damn looks like something is broken for linux powerpc

http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/21208/steps/ninja%20check%201/logs/stdio

llvm-lit: /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/projects/compiler-rt/test/builtins/Unit/lit.cfg.py:116: fatal: builtins_source_features contains duplicates: ['librt_has_divtc3']

I'm going to turn this into a warning to unbreak the bot for now.

@joerg @rupprecht Any idea what's going on here? My CMake code jsut reads the source files used to built the builtin library. My guess is that both lib/builtins/ppc/divtc3.c and lib/builtins/divtc3.c are used to build the builtin library for powerpc. That sounds like a bug. I'd expect that to cause a linker error with multiple symbols of the same name being defined.

[delcypher]

Landed workaround to downgrade fatal error to warning in r375162.

[delcypher]

@joerg @rupprecht

So lib/builtins/CMakeLists.txt has

set(GENERIC_SOURCES
  absvdi2.c
  ....
  divtc3.c
  ...
)

and then later

set(powerpc64_SOURCES
  ppc/divtc3.c
  ppc/fixtfdi.c
  ppc/fixunstfti.c
  ppc/fixunstfdi.c
  ppc/floattitf.c
  ppc/floatditf.c
  ppc/floatunditf.c
  ppc/gcc_qadd.c
  ppc/gcc_qdiv.c
  ppc/gcc_qmul.c
  ppc/gcc_qsub.c
  ppc/multc3.c
  ${GENERIC_SOURCES}
)

so it looks like both lib/builtins/divtc3.c and lib/builtins/ppc/divtc3.c are used for the builtin library for powerpc. I'm guessing rather than using set(powerpc64_SOURCES ... ${GENERIC_SOURCES}) we need a function that produces a list with files with duplicate filenames (not full path) removed.

[rupprecht]

In D68064#1713535, @delcypher wrote:

Damn looks like something is broken for linux powerpc

http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/21208/steps/ninja%20check%201/logs/stdio

llvm-lit: /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/projects/compiler-rt/test/builtins/Unit/lit.cfg.py:116: fatal: builtins_source_features contains duplicates: ['librt_has_divtc3']

I'm going to turn this into a warning to unbreak the bot for now.

@joerg @rupprecht Any idea what's going on here? My CMake code jsut reads the source files used to built the builtin library. My guess is that both lib/builtins/ppc/divtc3.c and lib/builtins/divtc3.c are used to build the builtin library for powerpc. That sounds like a bug. I'd expect that to cause a linker error with multiple symbols of the same name being defined.

I think you have the right idea. From my reading:

1. All the builtins are added to GENERIC_SOURCES: e.g. divtc3 here: https://github.com/llvm/llvm-project/blob/2ca8e27bd038673b514c8452d661a413ce0d2123/compiler-rt/lib/builtins/CMakeLists.txt#L68
2. Per-arch builtins get appended to that list: e.g. for ppc/divtc3 here: https://github.com/llvm/llvm-project/blob/2ca8e27bd038673b514c8452d661a413ce0d2123/compiler-rt/lib/builtins/CMakeLists.txt#L521
3. Generic versions are later filtered out if there's a arch-specific builtin: https://github.com/llvm/llvm-project/blob/2ca8e27bd038673b514c8452d661a413ce0d2123/compiler-rt/lib/builtins/CMakeLists.txt#L607

If that's where the sources is coming from, why is it not being filtered when this cmake config looks at it? Is it not running because of the if (APPLE) before it?

(Huge caveat: I spend almost no time developing in cmake, so I don't really know what I'm looking at)

[delcypher]

Hmm this code looks a little dubious. It's modifying a list while iterating over it and I'm not sure that regex is right.

# Filter out generic versions of routines that are re-implemented in
      # architecture specific manner.  This prevents multiple definitions of the
      # same symbols, making the symbol selection non-deterministic.
      foreach (_file ${${arch}_SOURCES})
        if (${_file} MATCHES ${_arch}/*)
          get_filename_component(_name ${_file} NAME)
          string(REPLACE ".S" ".c" _cname "${_name}")
          list(REMOVE_ITEM ${arch}_SOURCES ${_cname})
        endif ()
      endforeach ()

[delcypher]

Okay I figured out the issue. The problem is with the filtering code. It assumes that architecture specific specializations have a filepath like <arch>/filename. The problem here is that for powerpc64 the <arch> is powerpc64 but the directory name used is actually ppc. I've got a fix for this which should prevent people from making the same mistake in the future.

[delcypher]

Fix for powerpc issue is now up for review: https://reviews.llvm.org/D69189

test/builtins/CMakeLists.txt

if (${arch} STREQUAL "armhf")
     string(REPLACE ";" " " BUILTINS_TEST_TARGET_CFLAGS "${BUILTINS_TEST_TARGET_CFLAGS}")
   endif()
 
   if (${arch} STREQUAL "riscv32")
     list(APPEND BUILTINS_TEST_TARGET_CFLAGS -fforce-enable-int128)
     string(REPLACE ";" " " BUILTINS_TEST_TARGET_CFLAGS "${BUILTINS_TEST_TARGET_CFLAGS}")
   endif()
 
+  # Compute builtins available in library and add them as lit features.
+  if(APPLE)
+    # TODO: Support other Apple platforms.
+    set(BUILTIN_LIB_TARGET_NAME "clang_rt.builtins_${arch}_osx")
+  else()
+    set(BUILTIN_LIB_TARGET_NAME "clang_rt.builtins-${arch}")
+  endif()
+  if (NOT TARGET "${BUILTIN_LIB_TARGET_NAME}")
+    message(FATAL_ERROR "Target ${BUILTIN_LIB_TARGET_NAME} does not exist")
+  endif()
+  get_target_property(BUILTIN_LIB_SOURCES "${BUILTIN_LIB_TARGET_NAME}" SOURCES)
+  list(LENGTH BUILTIN_LIB_SOURCES BUILTIN_LIB_SOURCES_LENGTH)
+  if (BUILTIN_LIB_SOURCES_LENGTH EQUAL 0)
+    message(FATAL_ERROR "Failed to find source files for ${arch} builtin library")
+  endif()
+  set(BUILTINS_LIT_SOURCE_FEATURES "")
+  foreach (file_name ${BUILTIN_LIB_SOURCES})
+    # Strip off any directories and file extensions. This approach means we add
+    # add a single feature if there is a C source file or assembly override
+    # present in the builtin library.
+    # E.g.
+    # "hexagon/udivsi3.S" => "udivsi3"
+    # "udivsi3.c" => "udivsi3"
+    get_filename_component(FILE_NAME_FILTERED "${file_name}" NAME_WE)
+    list(APPEND BUILTINS_LIT_SOURCE_FEATURES "crt_has_${FILE_NAME_FILTERED}")
+  endforeach()
+
   string(TOUPPER ${arch} ARCH_UPPER_CASE)
   set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)
   configure_lit_site_cfg(
     ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.py.in
     ${CMAKE_CURRENT_BINARY_DIR}/Unit/${CONFIG_NAME}/lit.site.cfg.py
     )
   list(APPEND BUILTINS_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/Unit/${CONFIG_NAME})
 endforeach()
 
 # TODO: Add support for running tests on iOS and iOS simulator.
 
 add_lit_testsuite(check-builtins "Running the Builtins tests"
   ${BUILTINS_TESTSUITES}
   DEPENDS ${BUILTINS_TEST_DEPS})
 set_target_properties(check-builtins PROPERTIES FOLDER "Compiler-RT Misc")

test/builtins/Unit/comparedf2_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_comparedf2
 
 //===-- cmpdf2_test.c - Test __cmpdf2 -------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//

test/builtins/Unit/comparesf2_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_comparesf2
 
 //===-- cmpsf2_test.c - Test __cmpsf2 -------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//

test/builtins/Unit/divdf3_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_divdf3
 //===--------------- divdf3_test.c - Test __divdf3 ------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

test/builtins/Unit/divmodsi4_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_divmodsi4
 //===-- divmodsi4_test.c - Test __divmodsi4 -------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

test/builtins/Unit/divsf3_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_divsf3
 //===--------------- divsf3_test.c - Test __divsf3 ------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

test/builtins/Unit/divsi3_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_divsi3
 //===-- divsi3_test.c - Test __divsi3 -------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

test/builtins/Unit/lit.cfg.py

 config.substitutions.append( ("%arm_call_apsr ", \
                               build_invocation(call_apsr_flags)) )
 
 # Default test suffixes.
 config.suffixes = ['.c', '.cpp']
 
 if not config.emulator:
   config.available_features.add('native-run')
+
+# Add features for available sources
+builtins_source_features = config.builtins_lit_source_features.split(';')
+# Sanity checks
+if len(builtins_source_features) == 0:
+  lit_config.fatal('builtins_source_features cannot be empty')
+builtins_source_features_set = set()
+builtins_source_feature_duplicates = []
+for builtin_source_feature in builtins_source_features:
+  if len(builtin_source_feature) == 0:
+    lit_config.fatal('builtins_source_features cannot contain empty features')
+  old_size = len(builtins_source_features_set)
+  builtins_source_features_set.add(builtin_source_feature)
+  if len(builtins_source_features_set) == old_size:
+    builtins_source_feature_duplicates.append(builtin_source_feature)
+if len(builtins_source_feature_duplicates) > 0:
+  lit_config.fatal(
+    'builtins_source_features contains duplicates: {}'.format(
+      builtins_source_feature_duplicates)
+  )
+config.available_features.update(builtins_source_features)

test/builtins/Unit/lit.site.cfg.py.in

 @LIT_SITE_CFG_IN_HEADER@
 
 config.name_suffix = "@BUILTINS_TEST_CONFIG_SUFFIX@"
 config.builtins_lit_source_dir = "@BUILTINS_LIT_SOURCE_DIR@/Unit"
 config.target_cflags = "@BUILTINS_TEST_TARGET_CFLAGS@"
 config.target_arch = "@BUILTINS_TEST_TARGET_ARCH@"
 config.is_msvc = @MSVC_PYBOOL@
 config.builtins_is_msvc = @BUILTINS_IS_MSVC_PYBOOL@
+config.builtins_lit_source_features = "@BUILTINS_LIT_SOURCE_FEATURES@"
+
 # Load common config for all compiler-rt lit tests.
 lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured")
 
 # Load tool-specific config that would do the real work.
 lit_config.load_config(config, "@BUILTINS_LIT_SOURCE_DIR@/Unit/lit.cfg.py")

test/builtins/Unit/modsi3_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_modsi3
 /* ===-- modsi3_test.c - Test __modsi3 -------------------------------------===
  *
  * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  * See https://llvm.org/LICENSE.txt for license information.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
  *
  * ===----------------------------------------------------------------------===
  *

test/builtins/Unit/truncdfsf2_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_truncdfsf2
 
 //===--------------- truncdfsf2_test.c - Test __truncdfsf2 ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//

test/builtins/Unit/udivmodsi4_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_udivmodsi4
 //===-- udivmodsi4_test.c - Test __udivmodsi4 -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

test/builtins/Unit/udivsi3_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_udivsi3
 //===-- udivsi3_test.c - Test __udivsi3 -----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

test/builtins/Unit/umodsi3_test.c

 // RUN: %clang_builtins %s %librt -o %t && %run %t
+// REQUIRES: crt_has_umodsi3
 //===-- umodsi3_test.c - Test __umodsi3 -----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //