[clang-tools-extra] [clangd] Split huge generated CompletionModel.cpp into smaller files

[thesamesam]

Summary

Previously build system generated single huge CompletionModel.cpp with size about 6.5 MiB.
Compilation + assembling on PPC32 with GCC + GNU assembler resulted in 22010 errors:

{standard input}: Assembler messages:
{standard input}:181155: Error: operand out of range (0x0000000000008164 is not between 0xffffffffffff8000 and 0x0000000000007fff)
{standard input}:181192: Error: operand out of range (0x000000000000816c is not between 0xffffffffffff8000 and 0x0000000000007fff)
{standard input}:181211: Error: operand out of range (0x0000000000008080 is not between 0xffffffffffff8000 and 0x0000000000007fff)
{standard input}:181220: Error: operand out of range (0x00000000000081b8 is not between 0xffffffffffff8000 and 0x0000000000007fff)
{standard input}:181225: Error: operand out of range (0x0000000000008168 is not between 0xffffffffffff8000 and 0x0000000000007fff)
[...]

Separate compilation + assembling of given portion of code avoids exceeding
capabilities of compiler / assembler.

Bug: https://bugs.gentoo.org/829602
Thanks-to: Arfrever Frehtes Taifersar Arahesis <[e-mail]>
Tested-by: [e-mail] <[e-mail]>

[mgorny]

clang-tools-extra/clangd/CMakeLists.txt

48

Since the file is included, can't you just pass it via variable (i.e. set with PARENT_SCOPE)?

[thesamesam]

Use PARENT_SCOPE.

[thakis]

clang-tools-extra/clangd/quality/CompletionModel.cmake

25

Can we list them? We try not to use globs in cmakelists.txt files.

[Arfrever]

clang-tools-extra/clangd/quality/CompletionModel.cmake

25

This gen_decision_forest() function is called from 2 places (clang-tools-extra/clangd/CMakeLists.txt, clang-tools-extra/clangd/unittests/CMakeLists.txt), on different models, so this is impossible to do, at least in this file (clang-tools-extra/clangd/quality/CompletionModel.cmake).

And hardcoding of list of files in those CMakeLists.txt files would be inflexible and more fragile, since gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/quality/model CompletionModel ...) generates 301 .cpp files (1 file for each of 300 trees: CompletionModel0.cpp, CompletionModel1.cpp, ... until CompletionModel299.cpp, and 1 file for final function: CompletionModel.cpp).

[sammccall]

I don't think we should add significant build-system complexity here in order to support the completion model on ppc32.
Do people actually use clangd on ppc32 machines? (The linked bug calls this a clang build failure, but this code is not part of clang).

If we need to support building on this toolchain, then we should probably just disable the decision forest model entirely and use the heuristic completion scorer instead.

[thesamesam]

In D136283#3911832, @sammccall wrote:

I don't think we should add significant build-system complexity here in order to support the completion model on ppc32.
Do people actually use clangd on ppc32 machines? (The linked bug calls this a clang build failure, but this code is not part of clang).

If we need to support building on this toolchain, then we should probably just disable the decision forest model entirely and use the heuristic completion scorer instead.

I've had a think about this and I'm not aware of anyone using clangd on ppc32. So I agree this patch isn't worth the complexity. But it'd be nice to just switch to the heuristic scorer if it's somewhat easy rather than leave it broken (or bother having to wire up some "PPC is not supported" message). I've got no idea how to do that though.

[thakis]

I think this is obsolete now that D139107 is in, right? Marking as "request changes" to get it off my dashboard. Ideally you'll hit "Abandon" (which I can't do since I'm not owner of the patch).

[thesamesam]

Done, thanks!

clang-tools-extra/clangd/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   AllTargetsInfos
   FrontendOpenMP
   Option
   )
 
 include(${CMAKE_CURRENT_SOURCE_DIR}/quality/CompletionModel.cmake)
-gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/quality/model CompletionModel clang::clangd::Example)
+gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/quality/model
+  CompletionModel
+  clang::clangd::Example
+  CompletionModel_header_file
+  CompletionModel_cpp_files)
 
 if(MSVC AND NOT CLANG_CL)
  set_source_files_properties(CompileCommands.cpp PROPERTIES COMPILE_FLAGS -wd4130) # disables C4130: logical operation on address of string constant
 endif()
 
 include_directories(BEFORE "${CMAKE_CURRENT_BINARY_DIR}/../clang-tidy")
 
 add_clang_library(clangDaemon
add_clang_library(clangDaemon
   SemanticHighlighting.cpp
   SemanticSelection.cpp
   SourceCode.cpp
   QueryDriverDatabase.cpp
   TidyProvider.cpp
   TUScheduler.cpp
   URI.cpp
   XRefs.cpp
-  ${CMAKE_CURRENT_BINARY_DIR}/CompletionModel.cpp
+  ${CompletionModel_cpp_files}
 
   index/Background.cpp
   index/BackgroundIndexLoader.cpp
   index/BackgroundIndexStorage.cpp
   index/BackgroundQueue.cpp
   index/BackgroundRebuild.cpp
   index/CanonicalIncludes.cpp
   index/FileIndex.cpp
add_clang_library(clangDaemon
   refactor/InsertionPoint.cpp
   refactor/Rename.cpp
   refactor/Tweak.cpp
 
   DEPENDS
   omp_gen
   )
 
-# Include generated CompletionModel headers.
+# Include generated Completion Model header.
 target_include_directories(clangDaemon PUBLIC
-  $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
+  $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/CompletionModel>
 )
 
 clang_target_link_libraries(clangDaemon
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangDriver

clang-tools-extra/clangd/quality/CompletionModel.cmake

-# Run the Completion Model Codegenerator on the model present in the 
+# Run the Completion Model Codegenerator on the model present in the
 # ${model} directory.
-# Produces a pair of files called ${filename}.h and  ${filename}.cpp in the 
-# ${CMAKE_CURRENT_BINARY_DIR}. The generated header
+# Produces files called ${filename}.h and ${filename}*.cpp in the
+# ${CMAKE_CURRENT_BINARY_DIR}/${filename} directory. The generated header
 # will define a C++ class called ${cpp_class} - which may be a
 # namespace-qualified class name.
+# Sets variables referenced by ${output_header_file_variable} and
+# ${output_cpp_files_variable}.
 set(CLANGD_COMPLETION_MODEL_COMPILER ${CMAKE_CURRENT_LIST_DIR}/CompletionModelCodegen.py)
-function(gen_decision_forest model filename cpp_class)
+function(gen_decision_forest model filename cpp_class
+  output_header_file_variable output_cpp_files_variable)
   set(model_compiler ${CLANGD_COMPLETION_MODEL_COMPILER})
 
-  set(output_dir ${CMAKE_CURRENT_BINARY_DIR})
+  set(output_dir ${CMAKE_CURRENT_BINARY_DIR}/${filename})
   set(header_file ${output_dir}/${filename}.h)
-  set(cpp_file ${output_dir}/${filename}.cpp)
 
-  add_custom_command(OUTPUT ${header_file} ${cpp_file}
+  # Command for initial (CMake-invocation-time) generation of files.
+  message(STATUS "Generating code completion model runtime")
+  execute_process(COMMAND "${Python3_EXECUTABLE}" ${model_compiler}
+      --model ${model}
+      --output_dir ${output_dir}
+      --filename ${filename}
+      --cpp_class ${cpp_class})
+
+  file(GLOB cpp_files "${output_dir}/${filename}*.cpp")
+
+  # Command for potential regeneration of files after changes in model compiler,
+  # forest.json or features.json.
+  # This works reliably only if list of generated files is unchanged.
+  add_custom_command(OUTPUT ${header_file} ${cpp_files}
     COMMAND "${Python3_EXECUTABLE}" ${model_compiler}
       --model ${model}
       --output_dir ${output_dir}
       --filename ${filename}
       --cpp_class ${cpp_class}
     COMMENT "Generating code completion model runtime..."
     DEPENDS ${model_compiler} ${model}/forest.json ${model}/features.json
-    VERBATIM )
+    VERBATIM)
 
   set_source_files_properties(${header_file} PROPERTIES
     GENERATED 1)
-  set_source_files_properties(${cpp_file} PROPERTIES
+  set_source_files_properties(${cpp_files} PROPERTIES
     GENERATED 1)
 
   # Disable unused label warning for generated files.
   if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
-    set_source_files_properties(${cpp_file} PROPERTIES
+    set_source_files_properties(${cpp_files} PROPERTIES
       COMPILE_FLAGS /wd4102)
   else()
-    set_source_files_properties(${cpp_file} PROPERTIES
+    set_source_files_properties(${cpp_files} PROPERTIES
       COMPILE_FLAGS -Wno-unused)
   endif()
+
+  set(${output_header_file_variable} ${header_file} PARENT_SCOPE)
+  set(${output_cpp_files_variable} ${cpp_files} PARENT_SCOPE)
 endfunction()

clang-tools-extra/clangd/quality/CompletionModelCodegen.py

 """Code generator for Code Completion Model Inference.
 
 Tool runs on the Decision Forest model defined in {model} directory.
-It generates two files: {output_dir}/{filename}.h and {output_dir}/{filename}.cpp
-The generated files defines the Example class named {cpp_class} having all the features as class members.
+It generates files: {output_dir}/{filename}.h, {output_dir}/{filename}.cpp,
+and {output_dir}/{filename}{number}.cpp for each Decision Tree.
+The generated files define the Example class named {cpp_class} having all the features as class members.
 The generated runtime provides an `Evaluate` function which can be used to score a code completion candidate.
 """
 
 import argparse
 import json
+import os
 import struct
 
 
 class CppClass:
     """Holds class name and names of the enclosing namespaces."""
 
     def __init__(self, cpp_class):
         ns_and_class = cpp_class.split("::")
def order_encode(v):
     return TopBit + i  # top half of integers
 
 
 def evaluate_func(forest_json, cpp_class):
     """Generates evaluation functions for each tree and combines them in
     `float Evaluate(const {Example}&)` function. This function can be 
     used to score an Example."""
 
-    code = ""
+    functions_codes = {}
 
     # Generate evaluation function of each tree.
-    code += "namespace {\n"
     tree_num = 0
     for tree_json in forest_json:
-        code += "LLVM_ATTRIBUTE_NOINLINE float EvaluateTree%d(const %s& E) {\n" % (tree_num, cpp_class.name)
-        code += "  " + \
+        functions_codes[f"{tree_num}"] = "LLVM_ATTRIBUTE_NOINLINE float EvaluateTree%d(const %s& E) {\n" % (tree_num, cpp_class.name)
+        functions_codes[f"{tree_num}"] += "  " + \
             "\n  ".join(
                 tree(tree_json, tree_num=tree_num, node_num=0)[0]) + "\n"
-        code += "}\n\n"
+        functions_codes[f"{tree_num}"] += "}\n"
         tree_num += 1
-    code += "} // namespace\n\n"
 
     # Combine the scores of all trees in the final function.
     # MSAN will timeout if these functions are inlined.
-    code += "float Evaluate(const %s& E) {\n" % cpp_class.name
-    code += "  float Score = 0;\n"
+    final_function_code = ""
     for tree_num in range(len(forest_json)):
-        code += "  Score += EvaluateTree%d(E);\n" % tree_num
-    code += "  return Score;\n"
-    code += "}\n"
+        final_function_code += "float EvaluateTree%d(const %s& E);\n" % (tree_num, cpp_class.name)
+    final_function_code += "\n"
+    final_function_code += "float Evaluate(const %s& E) {\n" % cpp_class.name
+    final_function_code += "  float Score = 0;\n"
+    for tree_num in range(len(forest_json)):
+        final_function_code += "  Score += EvaluateTree%d(E);\n" % tree_num
+    final_function_code += "  return Score;\n"
+    final_function_code += "}\n"
 
-    return code
+    return functions_codes, final_function_code
 
 
 def gen_cpp_code(forest_json, features_json, filename, cpp_class):
-    """Generates code for the .cpp file."""
+    """Generates code for the .cpp files."""
     # Headers
     # Required by OrderEncode(float F).
     angled_include = [
         '#include <%s>' % h
         for h in ["cstring", "limits"]
     ]
 
     # Include generated header.
     qouted_headers = {filename + '.h', 'llvm/ADT/bit.h'}
     # Headers required by ENUM features used by the model.
     qouted_headers |= {f["header"]
                        for f in features_json if f["kind"] == "ENUM"}
     quoted_include = ['#include "%s"' % h for h in sorted(qouted_headers)]
 
     # using-decl for ENUM features.
     using_decls = "\n".join("using %s_type = %s;" % (
         feature['name'], feature['type'])
         for feature in features_json
         if feature["kind"] == "ENUM")
     nl = "\n"
-    return """%s
 
-%s
+    functions_codes, final_function_code = evaluate_func(forest_json, cpp_class)
 
-#define BIT(X) (1LL << X)
+    cpp_code = {
+        "": """%s
 
 %s
 
 %s
 
 uint32_t %s::OrderEncode(float F) {
   static_assert(std::numeric_limits<float>::is_iec559, "");
   constexpr uint32_t TopBit = ~(~uint32_t{0} >> 1);
 
   // Get the bits of the float. Endianness is the same as for integers.
   uint32_t U = llvm::bit_cast<uint32_t>(F);
   std::memcpy(&U, &F, sizeof(U));
   // IEEE 754 floats compare like sign-magnitude integers.
   if (U & TopBit)    // Negative float.
     return 0 - U;    // Map onto the low half of integers, order reversed.
   return U + TopBit; // Positive floats map onto the high half of integers.
 }
 
 %s
 %s
 """ % (nl.join(angled_include), nl.join(quoted_include), cpp_class.ns_begin(),
-       using_decls, cpp_class.name, evaluate_func(forest_json, cpp_class),
-       cpp_class.ns_end())
+       cpp_class.name, final_function_code, cpp_class.ns_end())}
+
+    for function_code_num, function_code in functions_codes.items():
+        cpp_code[function_code_num] = """%s
+
+#define BIT(X) (1LL << X)
+
+%s
+
+%s
+
+%s
+%s
+""" % (nl.join(quoted_include), cpp_class.ns_begin(), using_decls,
+       function_code, cpp_class.ns_end())
+
+    return cpp_code
 
 
 def main():
     parser = argparse.ArgumentParser('DecisionForestCodegen')
     parser.add_argument('--filename', help='output file name.')
     parser.add_argument('--output_dir', help='output directory.')
     parser.add_argument('--model', help='path to model directory.')
     parser.add_argument(
         '--cpp_class',
         help='The name of the class (which may be a namespace-qualified) created in generated header.'
     )
     ns = parser.parse_args()
 
     output_dir = ns.output_dir
     filename = ns.filename
     header_file = "%s/%s.h" % (output_dir, filename)
-    cpp_file = "%s/%s.cpp" % (output_dir, filename)
     cpp_class = CppClass(cpp_class=ns.cpp_class)
 
     model_file = "%s/forest.json" % ns.model
     features_file = "%s/features.json" % ns.model
 
     with open(features_file) as f:
         features_json = json.load(f)
 
     with open(model_file) as m:
         forest_json = json.load(m)
 
-    with open(cpp_file, 'w+t') as output_cc:
-        output_cc.write(
-            gen_cpp_code(forest_json=forest_json,
-                         features_json=features_json,
-                         filename=filename,
-                         cpp_class=cpp_class))
+    os.makedirs(output_dir, exist_ok=True)
+
+    for cpp_code_num, cpp_code in gen_cpp_code(forest_json=forest_json,
+        cpp_class=cpp_class).items():
+        cpp_file = "%s/%s%s.cpp" % (output_dir, filename, cpp_code_num)
+        with open(cpp_file, 'w+t') as output_cc:
+            output_cc.write(cpp_code)
 
     with open(header_file, 'w+t') as output_h:
         output_h.write(gen_header_code(
             features_json=features_json,
             cpp_class=cpp_class,
             filename=filename))
 
 
 if __name__ == '__main__':
     main()

clang-tools-extra/clangd/unittests/CMakeLists.txt

 
 if (CLANGD_ENABLE_REMOTE)
   include_directories(${CMAKE_CURRENT_BINARY_DIR}/../index/remote)
   add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI=1)
   set(REMOTE_TEST_SOURCES remote/MarshallingTests.cpp)
 endif()
 
 include(${CMAKE_CURRENT_SOURCE_DIR}/../quality/CompletionModel.cmake)
-gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/decision_forest_model DecisionForestRuntimeTest ::ns1::ns2::test::Example)
+gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/decision_forest_model
+  DecisionForestRuntimeTest
+  ::ns1::ns2::test::Example
+  DecisionForestRuntimeTest_header_file
+  DecisionForestRuntimeTest_cpp_files)
 
 add_custom_target(ClangdUnitTests)
 add_unittest(ClangdUnitTests ClangdTests
   Annotations.cpp
   ASTTests.cpp
   ASTSignalsTests.cpp
   BackgroundIndexTests.cpp
   CallHierarchyTests.cpp
add_unittest(ClangdUnitTests ClangdTests
   TestIndex.cpp
   TestTU.cpp
   TestWorkspace.cpp
   ThreadCrashReporterTests.cpp
   TidyProviderTests.cpp
   TypeHierarchyTests.cpp
   URITests.cpp
   XRefsTests.cpp
-  ${CMAKE_CURRENT_BINARY_DIR}/DecisionForestRuntimeTest.cpp
+  ${DecisionForestRuntimeTest_cpp_files}
 
   support/CancellationTests.cpp
   support/ContextTests.cpp
   support/FunctionTests.cpp
   support/MarkupTests.cpp
   support/MemoryTreeTests.cpp
   support/PathTests.cpp
   support/ThreadingTests.cpp
add_unittest(ClangdUnitTests ClangdTests
   tweaks/TweakTesting.cpp
   tweaks/TweakTests.cpp
 
   ${REMOTE_TEST_SOURCES}
 
   $<TARGET_OBJECTS:obj.clangDaemonTweaks>
   )
 
-# Include generated ComletionModel headers.
+# Include generated Completion Model header.
 target_include_directories(ClangdTests PUBLIC
-  $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
+  $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/DecisionForestRuntimeTest>
 )
 
 clang_target_link_libraries(ClangdTests
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangFormat