WIP: Redesign targets #2743

nurmukhametov · 2024-01-26T18:44:30Z

Design document

The ISPC compiler comes equipped with a standard library, named stdlib.ispc. By design, it has to be provided automatically for every compiled ISPC program unless the opposite said by --nostdlib command-line flag. This automatic inclusion ensures that essential functions—ranging from mathematical operations, print capabilities for output, memory management, to data conversion—are readily available to the developer.

The ISPC compiler also has an internal built-in library composed of three distinct sections:

Dispatcher: This component directs calls to the appropriate implemenation based on the actual ISA.
Target Independent: This includes functions that are universal across all target ISA, such as print.
Target Dependent: This part contains functions optimized for specific target ISA, like avx2 or avx512.

The standard and builtin libraries have to be distributed with ISPC compiler. It should automatically provide them taking into consideration target ISA and OS if needed. This design proposes to distribute them alongside the compiler as LLVM bitcode files. This approach requires the build system to prepare the libraries during the ISPC build phase as follows:

Dispatching and Target-Dependent Parts: These are written on templated LLVM IR and instantiated by m4 to produce target dependent code for every target ISA for any supported width (by current build configuration). Then llvm-as generates corresponding bitcode files.
Target-Independent Part: This is written in C/C++ and compiled by clang into bitcode format for every supported OS by current build configuration.
Standard Library: This is written in ISPC language and compiled into LLVM bitcode format by ISPC compiler, which must be pre-built.

ISPC compiles programs that can invoke functions from its standard library. A significant enhancement in this redesign is the introduction of the stdlib.isph header file, which outlines the definitions for the standard library's functionalities. This header can be included in ISPC programs either explicitly by the programmer or implicitly, except when the --nostdlib flag is provided. Furthermore, the stdlib.isph header introduces explicit definitions for essential symbols — such as programIndex, programCount, PI, _have.* and etc. —which were previously generated internally by the compiler using the LLVM API. This change also opens the door to implementing certain standard library functionalities directly within the header file using templates, e.g., short vector arithmetics.

All associated bitcode files from both the standard and built-in libraries, as well as the stdlib.isph header file, must be distributed with the ISPC compiler binary. There are two principal approaches:

Slim Binary: Distribute them alongside the ISPC binary within a designated directory structure.
Whole Binary: Embed them into the ISPC binary itself.

The first approach, unlike the second, introduces extra files and directories. These are essential for the ISPC compiler to accurately locate the bitcode files and the stdlib.isph header. Consequently, when distributing or relocating the ISPC binary, it's crucial to maintain the relative positions of these resources to ensure the compiler's functionality

The second approach is similar to the current method, where target-dependent bitcode files are embedded directly into the ISPC binary. This has the advantage of not affecting the current distribution practices, meaning ISPC users can continue downloading and installing ISPC with their existing workflows and scripts without any modifications.

It's worth noting that these approaches are not mutually exclusive. Some users, particularly those in the open-source community who maintain ISPC packages for different Linux OSes, may prefer the first approach.

The build process for ISPC should be adjusted to include the following steps:

Generation of all built-in library bitcode files.
Compilation of the slim ISPC binary.
Generation of standard library bitcode files by the slim ISPC binary.
Embedding all bitcode files and header files into the slim ISPC binary to produce the whole ISPC binary.

The last step is optional. The installation process should support both the slim and whole binary scenarios.

Compilation process

The compilation process of an ISPC program is structured into the following steps:

Adding declarations for both target-dependent and target-independent built-in functions to the ISPC symbol table.
Translating the ISPC source code into LLVM IR, which includes:
a. Implicitly including the stdlib.isph header if not explicitly included when needed,
b. Running the preprocessor on the ISPC source,
c. Parsing the ISPC source to create an internal AST representation,
d. Generating LLVM IR from the AST.
Adding definitions of standard library functions.
Adding definitions of target-independent builtin functions.
Adding definitions of target-dependent builtin functions.
Adding dispatcher definitions, if required.
Executing the optimization pipeline.
Utilizing the backend to generate the target code.

Steps 1 through 7 progressively populate the LLVM module with the necessary code. Step 2a ensures that symbols from the standard library utilized in the ISPC source file are resolved. The inclusion of symbols in Step 1 facilitates the resolution of built-in symbols within the standard library, while Steps 3 through 6 provide the actual function bodies for these symbols. In multitarget compilation scenarios, all built-in and standard library functions are appended with an ISA-specific suffix.

Prior to Step 8, it's essential to refine the LLVM module to exclude unused functions and symbols. This is accomplished by linking only the required symbols into the resulting module and performing global dead code elimination. An exception is made for a subset of functions, primarily related to memory access (e.g., gather/scatter operations), which must be retained throughout the optimization pipeline due to potential late-stage optimization uses (e.g., in ImproveMemoryOps.cpp). These are preserved through references in the llvm.compiler.used symbol and removed in the final optimization step by the RemovePersistentFuncs pass.

*Note about Step 1: Currently, this step involves uplifting LLVM IR declarations to ISPC symbol definitions, a process which can be seen as somewhat fragile and implicit. For example, the following declarations has no sense:

varying unsigned int32 __min_varying_int32(varying unsigned int32 , varying unsigned int32)
varying int32 __max_varying_uint32(varying int32 , varying int32).

Adopting an explicit declarative approach, possibly by defining all built-in functions in a builtin.isph header, would make Step 1 unnecessary. Moreover, this change could allow for the migration of various declarations (_pseudo*) and other target-independent functions from built-in LLVM files to the ISPC standard library code, enhancing clarity and simplification.

Hierarchical implementation of target-dependent builtin functions.

Ideally, built-in functions should have target independent implementation. It can be very useful for initial support of new targets, e.g. RISC-V or others. Implementing these target-independent built-in functions could be achieved through LLVM IR, avoiding target-specific constructs (like certain intrinsics), or possibly through ISPC itself, although its feasibility in the current state is uncertain. This common baseline has to be good enough as the starting point for implementing target specific and performance critical pieces in incremental manner. Taking the X86 ISA as an example, the development path of built-in functions could follow a progression from a common implementation to more specialized ones: common -> sse2 -> sse4 -> avx -> avx2 -> avx512.

The compilation process for ISPC programs, as outlined, necessitates adjustments to accommodate a hierarchical implementation of target built-in functions. This adjustment specifically impacts Step 5, which now requires additional effort. To integrate built-in code effectively, the process should initiate with the most specialized ISA compatible with the user's request. Should this ISA resolve all symbols, the process can advance to Step 6. If unresolved symbols remain, the process must ascend to a more general ISA, continuing to resolve symbols iteratively until all are addressed by a common implementation.
It's important to highlight that ISPC currently employs a similar strategy, utilizing manually written m4-based macro templates. However, this method necessitates that developers fully define all target-specific built-in libraries, a task that can be both time-consuming and prone to errors. This issue is likely pronounced for fragmented, feature-based ISAs like RISC-V.

dbabokin · 2024-02-03T03:06:12Z

I'll be looking at it over the weekend, but it wouldn't hurt to clean up the commit history - there are commits that just fix build or formatting.

But the bigger thing - would be good to have a design document for that. It's not just for making the review easier, but also to ensure that ideas behind the specific design are crisp and solid, and all trade offs and compromises are well documented.

aneshlya

In general the PR looks good! Before merging I'd like to see:

Design document
all TODOs implemented.
lit test checking the order and expected content after each Link* function - LinkDispatcher, LinkCommonBuiltins, LinkTargetBuiltins
I believe that lSetAsInternal should disappear

aneshlya · 2024-02-12T18:05:12Z

src/main.cpp

+    std::string ISPCExecutableAbsPath = llvm::sys::fs::getMainExecutable(argv[0], (void *)(intptr_t)main);
+    llvm::SmallString<128> includeDir(ISPCExecutableAbsPath);
+    llvm::sys::path::remove_filename(includeDir);
+    llvm::sys::path::remove_filename(includeDir);


Duplicated line?

aneshlya · 2024-02-12T18:56:02Z

src/bitcode_lib.cpp

+    llvm::SmallString<128> filePath(g->shareDirPath);
+    llvm::sys::path::append(filePath, m_filename);
+    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> bufferOrErr = llvm::MemoryBuffer::getFile(filePath.str());
+    // TODO! proper exit when load module fails


This TODO should be resolved before merging.

aneshlya · 2024-02-12T19:35:25Z

src/main.cpp

@@ -211,6 +211,7 @@ static void lPrintVersion() {
    printf("    [--[no-]discard-value-names]\tDo not discard/Discard value names when generating LLVM IR\n");
    printf("    [--dump-file[=<path>]]\t\tDump module IR to file(s) in "
           "current directory, or to <path> if specified\n");
+    printf("    [--gen-stdlib]\t\tEnable special compilation mode to generate LLVM IR for stdlib.ispc.");


is it supposed to be used when generating builtins from stdlib.ispc only? If yes, I would add it to description. Ideally I would check that stdlib.ispc is passed as input file. Now it's easy to think that it's some helping debug mode to explore/dump stdlib before inserting it to the user's module and can be used like ispc --gen-stdlib -o test.ll --emit-llvm-text test.ispc

aneshlya · 2024-02-12T19:41:08Z

.appveyor.yml

+  # Some builtins failed to build from the first time in parallel.
+  # It only happens with msbuild.
+    - cmd: |-
+        msbuild %ISPC_HOME%\build\builtins.vcxproj /p:Platform=x64 /p:Configuration=%configuration%


This line not needed if you build if you build with /m:1 below?

aneshlya · 2024-02-12T19:41:22Z

.appveyor.yml

@@ -64,7 +64,12 @@ for:
        mkdir build && cd build
        cmake .. -Thost=x64 -G %generator% -DCMAKE_BUILD_TYPE=%configuration% -DCMAKE_INSTALL_PREFIX=%ISPC_HOME%\install -DISPC_PREPARE_PACKAGE=ON -DM4_EXECUTABLE=C:\cygwin64\bin\m4.exe -DISPC_CROSS=ON -DISPC_GNUWIN32_PATH=%CROSS_TOOLS%\gnuwin32 -DWASM_ENABLED=%WASM% -DISPC_INCLUDE_BENCHMARKS=ON
  build_script:
-    - cmd: msbuild %ISPC_HOME%\build\PACKAGE.vcxproj /p:Platform=x64 /p:Configuration=%configuration%
+  # Some builtins failed to build from the first time in parallel.
+  # It only happens with msbuild.


Does it happen in Github Actions?

aneshlya · 2024-02-12T19:55:53Z

cmake/GenerateBuiltins.cmake

+                string(REGEX MATCH "^(xe|gen9)" isXe "${ispc_target}")
+                if (isXe)
+                    set(target_arch "xe64")
+                    if ("${bit}" STREQUAL "32")
+                        continue()
+                    endif()
+                endif()


Looks like it's the same as line 269-274

dbabokin · 2024-02-16T06:55:37Z

Would you mind rebasing this branch?

dbabokin

A couple observations while I'm still looking for general picture.

dbabokin · 2024-02-16T07:07:28Z

CMakeLists.txt

 )

+set(BITCODE_FOLDER ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}/../share/ispc)


I looked up CMAKE_CFG_INTDIR and it's Deprecated since version 3.21

dbabokin · 2024-02-16T07:17:53Z

stdlib.isph

+
+void memory_barrier();
+
+#define DEFINE_ATOMIC_OP(TA, TB, OPA, OPB, MASKTYPE, TC)                                                               \


Looks like it's not covered by clang-format.

nurmukhametov · 2024-03-11T18:07:00Z

But the bigger thing - would be good to have a design document for that. It's not just for making the review easier, but also to ensure that ideas behind the specific design are crisp and solid, and all trade offs and compromises are well documented.

See the design document added in the PR description.

aneshlya · 2024-03-12T23:58:34Z

I was thinking more about introducing of bitcode files to ISPC distribution and I see the following drawbacks:

Multiple versions mess up: It increases potential issues when people have several ISPC version installed on the system.
Security Risk: Exposed bitcode files might be more easily tampered with, posing a security risk.
Clutter: Can make the directory structure appear cluttered, and users might accidentally modify or delete necessary files.

But all of these can be more or less solved by wrapping them into static library.

But then I decided to look how people download and extract ISPC.
I reviewed the top 10 GitHub search results for "https://github.com/ispc/ispc/releases/download" and observed that users, including the OpenMoonray project, sometimes just copy bin/ispc directory in their installation scripts.

So changing of ISPC distribution model and adding a bunch of LLVM bitcode files in a share folder will break their scripts.

I suggest to list all pros/cons of having LLVM bitcode files separately vs embedded into the ISPC binary and see if the pros overcomes all the issues listed above.

nurmukhametov · 2024-03-22T12:48:14Z

I was thinking more about introducing of bitcode files to ISPC distribution and I see the following drawbacks:

Multiple versions mess up: It increases potential issues when people have several ISPC version installed on the system.

Security Risk: Exposed bitcode files might be more easily tampered with, posing a security risk.

Clutter: Can make the directory structure appear cluttered, and users might accidentally modify or delete necessary files.

My opinion is that the only actual drawback is security risk.

But then I decided to look how people download and extract ISPC. I reviewed the top 10 GitHub search results for "https://github.com/ispc/ispc/releases/download" and observed that users, including the OpenMoonray project, sometimes just copy bin/ispc directory in their installation scripts.

https://github.com/dreamworksanimation/openmoonray/blob/release/building/Rocky9/CMakeLists.txt#L72

https://github.com/supertuxkart/stk-code/blob/master/tools/linux_builder.sh#L169

These examples of installing ISPC just look wrong.

Notwithstanding the above, I have addressed that in the design document by the following change:

All associated bitcode files from both the standard and builtin libraries, as well as the stdlib.isph header file, must be distributed with the ISPC compiler binary within a designated directory structure. This directory structure is essential for the ISPC compiler to accurately locate these resources. Consequently, when distributing or relocating the ISPC binary, the bitcode files, and the stdlib.isph header file have to maintain their relative positions intact to ensure the compiler functionality.

to

All associated bitcode files from both the standard and built-in libraries, as well as the stdlib.isph header file, must be distributed with the ISPC compiler binary. There are two principal approaches:

Slim Binary: Distribute them alongside the ISPC binary within a designated directory structure.

Whole Binary: Embed them into the ISPC binary itself.

The first approach, unlike the second, introduces extra files and directories. These are essential for the ISPC compiler to accurately locate the bitcode files and the stdlib.isph header. Consequently, when distributing or relocating the ISPC binary, it's crucial to maintain the relative positions of these resources to ensure the compiler's functionality

The second approach is similar to the current method, where target-dependent bitcode files are embedded directly into the ISPC binary. This has the advantage of not affecting the current distribution practices, meaning ISPC users can continue downloading and installing ISPC with their existing workflows and scripts without any modifications.

It's worth noting that these approaches are not mutually exclusive. Some users, particularly those in the open-source community who maintain ISPC packages for different Linux OSes, may prefer the first approach.

The build process for ISPC should be adjusted to include the following steps:

Generation of all built-in library bitcode files.

Compilation of the slim ISPC binary.

Generation of standard library bitcode files by the slim ISPC binary.

Embedding all bitcode files and header files into the slim ISPC binary to produce the whole ISPC binary.

The last step is optional. The installation process should support both the slim and whole binary scenarios.

The PR message contains the updated design document.

nurmukhametov force-pushed the redesign-targets branch 4 times, most recently from 0f9d5b2 to fc536d4 Compare February 1, 2024 14:03

nurmukhametov force-pushed the redesign-targets branch 4 times, most recently from 7aa0012 to 6a73918 Compare February 12, 2024 13:09

aneshlya reviewed Feb 12, 2024

View reviewed changes

dbabokin reviewed Feb 16, 2024

View reviewed changes

Redesign code working with target specific code.

e20b513

nurmukhametov force-pushed the redesign-targets branch from 6a73918 to e20b513 Compare March 1, 2024 19:11

nurmukhametov mentioned this pull request Apr 1, 2024

Two stage build #2823

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Redesign targets #2743

WIP: Redesign targets #2743

nurmukhametov commented Jan 26, 2024 •

edited

dbabokin commented Feb 3, 2024

aneshlya left a comment

aneshlya Feb 12, 2024

aneshlya Feb 12, 2024

aneshlya Feb 12, 2024

aneshlya Feb 12, 2024

aneshlya Feb 12, 2024

aneshlya Feb 12, 2024

dbabokin commented Feb 16, 2024

dbabokin left a comment

dbabokin Feb 16, 2024

dbabokin Feb 16, 2024

nurmukhametov commented Mar 11, 2024

aneshlya commented Mar 12, 2024

nurmukhametov commented Mar 22, 2024

		)

		set(BITCODE_FOLDER ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}/../share/ispc)


		void memory_barrier();

		#define DEFINE_ATOMIC_OP(TA, TB, OPA, OPB, MASKTYPE, TC) \

WIP: Redesign targets #2743

Are you sure you want to change the base?

WIP: Redesign targets #2743

Conversation

nurmukhametov commented Jan 26, 2024 • edited

Design document

Compilation process

Hierarchical implementation of target-dependent builtin functions.

dbabokin commented Feb 3, 2024

aneshlya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbabokin commented Feb 16, 2024

dbabokin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nurmukhametov commented Mar 11, 2024

aneshlya commented Mar 12, 2024

nurmukhametov commented Mar 22, 2024

nurmukhametov commented Jan 26, 2024 •

edited