[libc-commits] [libc] 785d69e - [libc][test] make `str_to_float_comparison_test` independent of C++ headers. (#133978)

via libc-commits libc-commits at lists.llvm.org
Wed Apr 9 14:11:06 PDT 2025


Author: Muhammad Bassiouni
Date: 2025-04-09T14:11:02-07:00
New Revision: 785d69e317f249d96ca10010c96a92c51fa4d199

URL: https://github.com/llvm/llvm-project/commit/785d69e317f249d96ca10010c96a92c51fa4d199
DIFF: https://github.com/llvm/llvm-project/commit/785d69e317f249d96ca10010c96a92c51fa4d199.diff

LOG: [libc][test] make `str_to_float_comparison_test` independent of C++ headers. (#133978)

This is an attempt to move away from C++ headers to be able to run the
test without `libcxx`.
closes #129838

cc @lntue @RossComputerGuy

Added: 
    

Modified: 
    libc/cmake/modules/LLVMLibCTestRules.cmake
    libc/test/src/__support/CMakeLists.txt
    libc/test/src/__support/str_to_float_comparison_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 0f0d612d04ba5..63a8e9ecda002 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -191,6 +191,7 @@ endfunction(get_object_files_for_test)
 #      SRCS  <list of .cpp files for the test>
 #      HDRS  <list of .h files for the test>
 #      DEPENDS <list of dependencies>
+#      ENV <list of environment variables to set before running the test>
 #      COMPILE_OPTIONS <list of special compile options for this target>
 #      LINK_LIBRARIES <list of linking libraries for this target>
 #    )
@@ -203,7 +204,7 @@ function(create_libc_unittest fq_target_name)
     "LIBC_UNITTEST"
     "NO_RUN_POSTBUILD;C_TEST" # Optional arguments
     "SUITE;CXX_STANDARD" # Single value arguments
-    "SRCS;HDRS;DEPENDS;COMPILE_OPTIONS;LINK_LIBRARIES;FLAGS" # Multi-value arguments
+    "SRCS;HDRS;DEPENDS;ENV;COMPILE_OPTIONS;LINK_LIBRARIES;FLAGS" # Multi-value arguments
     ${ARGN}
   )
   if(NOT LIBC_UNITTEST_SRCS)
@@ -319,7 +320,7 @@ function(create_libc_unittest fq_target_name)
   if(NOT LIBC_UNITTEST_NO_RUN_POSTBUILD)
     add_custom_target(
       ${fq_target_name}
-      COMMAND ${fq_build_target_name}
+      COMMAND ${LIBC_UNITTEST_ENV} ${CMAKE_CROSSCOMPILING_EMULATOR} ${fq_build_target_name}
       COMMENT "Running unit test ${fq_target_name}"
     )
   endif()
@@ -642,7 +643,7 @@ function(add_libc_hermetic test_name)
   endif()
   cmake_parse_arguments(
     "HERMETIC_TEST"
-    "IS_GPU_BENCHMARK" # Optional arguments
+    "IS_GPU_BENCHMARK;NO_RUN_POSTBUILD" # Optional arguments
     "SUITE;CXX_STANDARD" # Single value arguments
     "SRCS;HDRS;DEPENDS;ARGS;ENV;COMPILE_OPTIONS;LINK_LIBRARIES;LOADER_ARGS" # Multi-value arguments
     ${ARGN}
@@ -713,7 +714,12 @@ function(add_libc_hermetic test_name)
   set_target_properties(${fq_target_name}.__libc__
       PROPERTIES ARCHIVE_OUTPUT_NAME ${fq_target_name}.libc)
 
-  set(fq_build_target_name ${fq_target_name}.__build__)
+  if(HERMETIC_TEST_NO_RUN_POSTBUILD)
+    set(fq_build_target_name ${fq_target_name})
+  else()
+    set(fq_build_target_name ${fq_target_name}.__build__)
+  endif()
+
   add_executable(
     ${fq_build_target_name}
     EXCLUDE_FROM_ALL
@@ -794,26 +800,28 @@ function(add_libc_hermetic test_name)
     get_target_property(gpu_loader_exe libc.utils.gpu.loader "EXECUTABLE")
   endif()
 
-  set(test_cmd ${HERMETIC_TEST_ENV}
-      $<$<BOOL:${LIBC_TARGET_OS_IS_GPU}>:${gpu_loader_exe}> ${CMAKE_CROSSCOMPILING_EMULATOR} ${HERMETIC_TEST_LOADER_ARGS}
-      $<TARGET_FILE:${fq_build_target_name}> ${HERMETIC_TEST_ARGS})
-  add_custom_target(
-    ${fq_target_name}
-    DEPENDS ${fq_target_name}-cmd
-  )
+  if(NOT HERMETIC_TEST_NO_RUN_POSTBUILD)
+    set(test_cmd ${HERMETIC_TEST_ENV}
+        $<$<BOOL:${LIBC_TARGET_OS_IS_GPU}>:${gpu_loader_exe}> ${CMAKE_CROSSCOMPILING_EMULATOR} ${HERMETIC_TEST_LOADER_ARGS}
+        $<TARGET_FILE:${fq_build_target_name}> ${HERMETIC_TEST_ARGS})
+    add_custom_target(
+      ${fq_target_name}
+      DEPENDS ${fq_target_name}.__cmd__
+    )
 
-  add_custom_command(
-    OUTPUT ${fq_target_name}-cmd
-    COMMAND ${test_cmd}
-    COMMAND_EXPAND_LISTS
-    COMMENT "Running hermetic test ${fq_target_name}"
-    ${LIBC_HERMETIC_TEST_JOB_POOL}
-  )
+    add_custom_command(
+      OUTPUT ${fq_target_name}.__cmd__
+      COMMAND ${test_cmd}
+      COMMAND_EXPAND_LISTS
+      COMMENT "Running hermetic test ${fq_target_name}"
+      ${LIBC_HERMETIC_TEST_JOB_POOL}
+    )
 
-  set_source_files_properties(${fq_target_name}-cmd
-    PROPERTIES
-      SYMBOLIC "TRUE"
-  )
+    set_source_files_properties(${fq_target_name}.__cmd__
+      PROPERTIES
+        SYMBOLIC "TRUE"
+    )
+  endif()
 
   add_dependencies(${HERMETIC_TEST_SUITE} ${fq_target_name})
   if(NOT ${HERMETIC_TEST_IS_GPU_BENCHMARK})

diff  --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index d056969034d69..b50bf245f202c 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -249,32 +249,25 @@ add_libc_test(
     libc.src.__support.memory_size
 )
 
-# FIXME: We shouldn't have regular executables created because we could be
-#        cross-compiling the tests and running through an emulator.
-if(NOT LIBC_TARGET_OS_IS_GPU)
-  add_executable(
-    libc_str_to_float_comparison_test
-    str_to_float_comparison_test.cpp
-  )
-
-  target_link_libraries(libc_str_to_float_comparison_test
-    PRIVATE
-      "${LIBC_TARGET}"
-  )
-
-  add_executable(
-    libc_system_str_to_float_comparison_test
+add_libc_test(
+  str_to_float_comparison_test
+  NO_RUN_POSTBUILD
+  SUITE
+    libc-support-tests
+  SRCS
     str_to_float_comparison_test.cpp
-  )
-
-  set(float_test_file ${CMAKE_CURRENT_SOURCE_DIR}/str_to_float_comparison_data.txt)
-
-  add_custom_command(TARGET libc_str_to_float_comparison_test
-                     POST_BUILD
-                     COMMAND ${CMAKE_CROSSCOMPILING_EMULATOR} $<TARGET_FILE:libc_str_to_float_comparison_test> ${float_test_file}
-                     COMMENT "Test the strtof and strtod implementations against precomputed results."
-                     VERBATIM)
-endif()
+  DEPENDS
+    libc.src.stdio.printf
+    libc.src.stdio.fopen
+    libc.src.stdio.fclose
+    libc.src.stdio.fgets
+    libc.src.stdlib.strtof
+    libc.src.stdlib.strtod
+    libc.src.stdlib.getenv
+    libc.src.string.strtok
+    libc.src.string.strdup
+    libc.src.__support.CPP.bit
+)
 
 add_subdirectory(CPP)
 add_subdirectory(File)

diff  --git a/libc/test/src/__support/str_to_float_comparison_test.cpp b/libc/test/src/__support/str_to_float_comparison_test.cpp
index 61bfc3cd0903a..6e89ce2aabf3e 100644
--- a/libc/test/src/__support/str_to_float_comparison_test.cpp
+++ b/libc/test/src/__support/str_to_float_comparison_test.cpp
@@ -6,16 +6,18 @@
 //
 //===----------------------------------------------------------------------===//
 
-// #include "src/__support/str_float_conv_utils.h"
-
-#include <stdlib.h> // For string to float functions
-
-// #include "src/__support/FPUtil/FPBits.h"
-
-#include <cstdint>
-#include <fstream>
-#include <iostream>
-#include <string>
+#include "src/__support/CPP/bit.h"
+#include "src/stdio/fclose.h"
+#include "src/stdio/fgets.h"
+#include "src/stdio/fopen.h"
+#include "src/stdio/printf.h"
+#include "src/stdlib/getenv.h"
+#include "src/stdlib/strtod.h"
+#include "src/stdlib/strtof.h"
+#include "src/string/strdup.h"
+#include "src/string/strtok.h"
+#include "test/UnitTest/Test.h"
+#include <stdint.h>
 
 // The intent of this test is to read in files in the format used in this test
 // dataset: https://github.com/nigeltao/parse-number-fxx-test-data
@@ -32,6 +34,19 @@
 // ./libc_str_to_float_comparison_test <path/to/dataset/repo>/data/*
 // It will take a few seconds to run.
 
+struct ParseResult {
+  uint32_t totalFails;
+  uint32_t totalBitDiffs;
+  uint32_t detailedBitDiffs[4];
+  uint32_t total;
+};
+
+enum class ParseStatus : uint8_t {
+  SUCCESS,
+  FILE_ERROR,
+  PARSE_ERROR,
+};
+
 static inline uint32_t hexCharToU32(char in) {
   return in > '9' ? in + 10 - 'A' : in - '0';
 }
@@ -54,120 +69,168 @@ static inline uint64_t fastHexToU64(const char *inStr) {
   return result;
 }
 
-int checkFile(char *inputFileName, int *totalFails, int *totalBitDiffs,
-              int *detailedBitDiffs, int *total) {
-  int32_t curFails = 0;    // Only counts actual failures, not bit
diff s.
-  int32_t curBitDiffs = 0; // A bit
diff  is when the expected result and actual
-                           // result are off by +/- 1 bit.
-  std::string line;
-  std::string num;
+static void parseLine(const char *line, ParseResult &parseResult,
+                      int32_t &curFails, int32_t &curBitDiffs) {
 
-  std::ifstream fileStream(inputFileName, std::ifstream::in);
+  if (line[0] == '#')
+    return;
 
-  if (!fileStream.is_open()) {
-    std::cout << "file '" << inputFileName << "' failed to open. Exiting.\n";
-    return 1;
-  }
-  while (getline(fileStream, line)) {
-    if (line[0] == '#') {
-      continue;
-    }
-    *total = *total + 1;
-    uint32_t expectedFloatRaw;
-    uint64_t expectedDoubleRaw;
+  parseResult.total += 1;
+  uint32_t expectedFloatRaw;
+  uint64_t expectedDoubleRaw;
 
-    expectedFloatRaw = fastHexToU32(line.c_str() + 5);
-    expectedDoubleRaw = fastHexToU64(line.c_str() + 14);
-    num = line.substr(31);
+  expectedFloatRaw = fastHexToU32(line + 5);
+  expectedDoubleRaw = fastHexToU64(line + 14);
 
-    float floatResult = strtof(num.c_str(), nullptr);
+  const char *num = line + 31;
 
-    double doubleResult = strtod(num.c_str(), nullptr);
+  float floatResult = LIBC_NAMESPACE::strtof(num, nullptr);
 
-    uint32_t floatRaw = *(uint32_t *)(&floatResult);
+  double doubleResult = LIBC_NAMESPACE::strtod(num, nullptr);
 
-    uint64_t doubleRaw = *(uint64_t *)(&doubleResult);
+  uint32_t floatRaw = LIBC_NAMESPACE::cpp::bit_cast<uint32_t>(floatResult);
 
-    if (!(expectedFloatRaw == floatRaw)) {
-      if (expectedFloatRaw == floatRaw + 1 ||
-          expectedFloatRaw == floatRaw - 1) {
-        curBitDiffs++;
-        if (expectedFloatRaw == floatRaw + 1) {
-          detailedBitDiffs[0] = detailedBitDiffs[0] + 1; // float low
-        } else {
-          detailedBitDiffs[1] = detailedBitDiffs[1] + 1; // float high
-        }
+  uint64_t doubleRaw = LIBC_NAMESPACE::cpp::bit_cast<uint64_t>(doubleResult);
+
+  if (!(expectedFloatRaw == floatRaw)) {
+    if (expectedFloatRaw == floatRaw + 1 || expectedFloatRaw == floatRaw - 1) {
+      curBitDiffs++;
+      if (expectedFloatRaw == floatRaw + 1) {
+        parseResult.detailedBitDiffs[0] =
+            parseResult.detailedBitDiffs[0] + 1; // float low
       } else {
-        curFails++;
-      }
-      if (curFails + curBitDiffs < 10) {
-        std::cout << "Float fail for '" << num << "'. Expected " << std::hex
-                  << expectedFloatRaw << " but got " << floatRaw << "\n"
-                  << std::dec;
+        parseResult.detailedBitDiffs[1] =
+            parseResult.detailedBitDiffs[1] + 1; // float high
       }
+    } else {
+      curFails++;
     }
+    if (curFails + curBitDiffs < 10) {
+      LIBC_NAMESPACE::printf("Float fail for '%s'. Expected %x but got %x\n",
+                             num, expectedFloatRaw, floatRaw);
+    }
+  }
 
-    if (!(expectedDoubleRaw == doubleRaw)) {
-      if (expectedDoubleRaw == doubleRaw + 1 ||
-          expectedDoubleRaw == doubleRaw - 1) {
-        curBitDiffs++;
-        if (expectedDoubleRaw == doubleRaw + 1) {
-          detailedBitDiffs[2] = detailedBitDiffs[2] + 1; // double low
-        } else {
-          detailedBitDiffs[3] = detailedBitDiffs[3] + 1; // double high
-        }
+  if (!(expectedDoubleRaw == doubleRaw)) {
+    if (expectedDoubleRaw == doubleRaw + 1 ||
+        expectedDoubleRaw == doubleRaw - 1) {
+      curBitDiffs++;
+      if (expectedDoubleRaw == doubleRaw + 1) {
+        parseResult.detailedBitDiffs[2] =
+            parseResult.detailedBitDiffs[2] + 1; // double low
       } else {
-        curFails++;
-      }
-      if (curFails + curBitDiffs < 10) {
-        std::cout << "Double fail for '" << num << "'. Expected " << std::hex
-                  << expectedDoubleRaw << " but got " << doubleRaw << "\n"
-                  << std::dec;
+        parseResult.detailedBitDiffs[3] =
+            parseResult.detailedBitDiffs[3] + 1; // double high
       }
+    } else {
+      curFails++;
+    }
+    if (curFails + curBitDiffs < 10) {
+      LIBC_NAMESPACE::printf("Double fail for '%s'. Expected %lx but got %lx\n",
+                             num, expectedDoubleRaw, doubleRaw);
     }
   }
+}
 
-  fileStream.close();
+ParseStatus checkBuffer(ParseResult &parseResult) {
+  constexpr const char *LINES[] = {
+      "3C00 3F800000 3FF0000000000000 1",
+      "3D00 3FA00000 3FF4000000000000 1.25",
+      "3D9A 3FB33333 3FF6666666666666 1.4",
+      "57B7 42F6E979 405EDD2F1A9FBE77 123.456",
+      "622A 44454000 4088A80000000000 789",
+      "7C00 7F800000 7FF0000000000000 123.456e789"};
+
+  int32_t curFails = 0;    // Only counts actual failures, not bit
diff s.
+  int32_t curBitDiffs = 0; // A bit
diff  is when the expected result and actual
+  // result are off by +/- 1 bit.
+
+  for (uint8_t i = 0; i < sizeof(LINES) / sizeof(LINES[0]); i++) {
+    parseLine(LINES[i], parseResult, curFails, curBitDiffs);
+  }
 
-  *totalBitDiffs += curBitDiffs;
-  *totalFails += curFails;
+  parseResult.totalBitDiffs += curBitDiffs;
+  parseResult.totalFails += curFails;
 
   if (curFails > 1 || curBitDiffs > 1) {
-    return 2;
+    return ParseStatus::PARSE_ERROR;
   }
-  return 0;
+  return ParseStatus::SUCCESS;
 }
 
-int main(int argc, char *argv[]) {
-  int result = 0;
-  int fails = 0;
+ParseStatus checkFile(char *inputFileName, ParseResult &parseResult) {
+  int32_t curFails = 0;    // Only counts actual failures, not bit
diff s.
+  int32_t curBitDiffs = 0; // A bit
diff  is when the expected result and actual
+  // result are off by +/- 1 bit.
+  char line[1000];
+
+  auto *fileHandle = LIBC_NAMESPACE::fopen(inputFileName, "r");
+
+  if (!fileHandle) {
+    LIBC_NAMESPACE::printf("file '%s' failed to open. Exiting.\n",
+                           inputFileName);
+    return ParseStatus::FILE_ERROR;
+  }
+
+  while (LIBC_NAMESPACE::fgets(line, sizeof(line), fileHandle)) {
+    parseLine(line, parseResult, curFails, curBitDiffs);
+  }
+
+  LIBC_NAMESPACE::fclose(fileHandle);
+
+  parseResult.totalBitDiffs += curBitDiffs;
+  parseResult.totalFails += curFails;
+
+  if (curFails > 1 || curBitDiffs > 1) {
+    return ParseStatus::PARSE_ERROR;
+  }
+  return ParseStatus::SUCCESS;
+}
+
+ParseStatus updateStatus(ParseStatus parse_status, ParseStatus cur_status) {
+  if (cur_status == ParseStatus::FILE_ERROR) {
+    parse_status = ParseStatus::FILE_ERROR;
+  } else if (cur_status == ParseStatus::PARSE_ERROR) {
+    parse_status = ParseStatus::PARSE_ERROR;
+  }
+  return parse_status;
+}
+
+TEST(LlvmLibcStrToFloatComparisonTest, CheckFloats) {
+  ParseStatus parseStatus = ParseStatus::SUCCESS;
 
   // Bit
diff s are cases where the expected result and actual result only 
diff er
   // by +/- the least significant bit. They are tracked separately from larger
   // failures since a bit
diff  is most likely the result of a rounding error, and
   // splitting them off makes them easier to track down.
-  int bit
diff s = 0;
-  int detailedBitDiffs[4] = {0, 0, 0, 0};
-
-  int total = 0;
-  for (int i = 1; i < argc; i++) {
-    std::cout << "Starting file " << argv[i] << "\n";
-    int curResult =
-        checkFile(argv[i], &fails, &bit
diff s, detailedBitDiffs, &total);
-    if (curResult == 1) {
-      result = 1;
-      break;
-    } else if (curResult == 2) {
-      result = 2;
+
+  ParseResult parseResult = {
+      .totalFails = 0,
+      .totalBitDiffs = 0,
+      .detailedBitDiffs = {0, 0, 0, 0},
+      .total = 0,
+  };
+
+  char *files = LIBC_NAMESPACE::getenv("FILES");
+
+  if (files == nullptr) {
+    ParseStatus cur_status = checkBuffer(parseResult);
+    parseStatus = updateStatus(parseStatus, cur_status);
+  } else {
+    files = LIBC_NAMESPACE::strdup(files);
+    for (char *file = LIBC_NAMESPACE::strtok(files, ","); file != nullptr;
+         file = LIBC_NAMESPACE::strtok(nullptr, ",")) {
+      ParseStatus cur_status = checkFile(file, parseResult);
+      parseStatus = updateStatus(parseStatus, cur_status);
     }
   }
-  std::cout << "Results:\n"
-            << "Total significant failed conversions: " << fails << "\n"
-            << "Total conversions off by +/- 1 bit: " << bit
diff s << "\n"
-            << "\t" << detailedBitDiffs[0] << "\tfloat low\n"
-            << "\t" << detailedBitDiffs[1] << "\tfloat high\n"
-            << "\t" << detailedBitDiffs[2] << "\tdouble low\n"
-            << "\t" << detailedBitDiffs[3] << "\tdouble high\n"
-            << "Total lines: " << total << "\n";
-  return result;
+
+  EXPECT_EQ(parseStatus, ParseStatus::SUCCESS);
+  EXPECT_EQ(parseResult.totalFails, 0u);
+  EXPECT_EQ(parseResult.totalBitDiffs, 0u);
+  EXPECT_EQ(parseResult.detailedBitDiffs[0], 0u); // float low
+  EXPECT_EQ(parseResult.detailedBitDiffs[1], 0u); // float high
+  EXPECT_EQ(parseResult.detailedBitDiffs[2], 0u); // double low
+  EXPECT_EQ(parseResult.detailedBitDiffs[3], 0u); // double high
+  LIBC_NAMESPACE::printf("Total lines: %d\n", parseResult.total);
 }


        


More information about the libc-commits mailing list