Two stage build #2823

nurmukhametov · 2024-04-01T12:40:09Z

This PR follows up the discussion in PR #2743 about redesigning targets.

This change follows the redesign design document as quoted here:

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 steps 1, 2, 4 (excluding non-existent yet header) are implemented here. Default build includes the last step and configures to build and install ispc-whole named as ispc.

In default configuration, ispc target corresponds to ispc-whole, i.e., behavior is as same as before.

Under ISPC_SLIM_BINARY=ON, ispc target corresponds to ispc-slim that does not contain builtin libraries inside binary, they are placed alongside as LLVM bitcode files.

New build targets are introduced:

dispatch-builtins,
common-builtins,
target-builtins,
builtins,
dispatch-builtins-cpp,
common-builtins-cpp,
target-builtins-cpp,
builtins-cpp,
common.

Generation of builtins-*.cpp are splitted to two stages:

generation of LLVM bitcode files (to use them for ispc-slim), corresponding targets (dispatch-builtins, common-builtins, target-builtins, builtins),
generation of CPP with embedded LLVM bitcode (to use them to compile ispc-whole), corresponding targets (dispatch-builtins-cpp, common-builtins-cpp, target-builtins-cpp, builtins-cpp).

turinevgeny · 2024-04-03T02:46:39Z

CMakeLists.txt

+    target_include_directories(${TARGET} PRIVATE ${INCLUDE_DIRECTORIES})
+    target_compile_options(${TARGET} PRIVATE ${COMPILE_OPTIONS})
+
+    # Set hidden visibility for inline functions.  This is needed to be in sync


Seems like a double space

Suggested change

# Set hidden visibility for inline functions. This is needed to be in sync

# Set hidden visibility for inline functions. This is needed to be in sync

turinevgeny · 2024-04-03T02:55:15Z

CMakeLists.txt

    endif()
 endif()

+# It may be confusing that some CMake variables start with ISPC but some not.


Does it make sense to leave a TODO: mark?

Yes, sure. I've added it.

CMakeLists.txt

turinevgeny · 2024-04-03T03:19:43Z

src/bitcode_lib.cpp

@@ -9,25 +9,56 @@
           Builtiins-c, or ISPCTarget).
 */

+#include <stdlib.h>


Do the ISPC sources adhere to the LLVM coding style of includes?
https://llvm.org/docs/CodingStandards.html#include-style

I am not sure. It looks like almost except system headers place.

turinevgeny · 2024-04-03T03:23:47Z

src/builtins.cpp

+    // FIXME: this feels like a bad idea, but the issue is that when we
+    // set the llvm::Module's target triple in the ispc Module::Module
+    // constructor, we start by calling llvm::sys::getHostTriple() (and
+    // then change the arch if needed).  Somehow that ends up giving us


Suggested change

// then change the arch if needed). Somehow that ends up giving us

// then change the arch if needed). Somehow that ends up giving us

turinevgeny · 2024-04-03T03:23:55Z

src/builtins.cpp

+    // then change the arch if needed).  Somehow that ends up giving us
+    // strings like 'x86_64-apple-darwin11.0.0', while the stuff we
+    // compile to bitcode with clang has module triples like
+    // 'i386-apple-macosx10.7.0'.  And then LLVM issues a warning about


Suggested change

// 'i386-apple-macosx10.7.0'. And then LLVM issues a warning about

// 'i386-apple-macosx10.7.0'. And then LLVM issues a warning about

turinevgeny · 2024-04-03T03:25:28Z

src/slim.cpp

+}
+
+// Dispatch bitcode_libs
+// TODO! strangly enough this TargetOS has no sense and this dispatch lib is used on windows


Suggested change

// TODO! strangly enough this TargetOS has no sense and this dispatch lib is used on windows

// TODO! strangely enough this TargetOS has no sense and this dispatch lib is used on windows

turinevgeny · 2024-04-03T03:26:17Z

src/slim.cpp

+
+// Target-specific built-in bitcode_libs
+// clang-format off
+#if defined(ISPC_UNIX_TARGET_ON) && defined(ISPC_X86_ENABLED)


Is it possible to simplify the patterns below with, maybe, macro magic?

Under assumption that target name in file contains _ instead of -, it is possible to simplify with something like this (not really tested though):

#define SSE2_TARGETS(M) \ M(sse2_i32x4); \ M(sse2_i32x8); #define SSE4_TARGETS(M) \ M(sse4_i8x16) \ M(sse4_i16x8) \ M(sse4_i32x4) \ M(sse4_i32x8) #define AVX1_TARGETS(M) \ M(avx1_i32x8) \ M(avx1_i32x16) \ M(avx1_i64x4) #define AVX2_TARGETS(M) \ M(avx2_i8x32) \ M(avx2_i16x16) \ M(avx2_i32x4) \ M(avx2_i32x8) \ M(avx2_i32x16) \ M(avx2_i64x4) #define AVX512KNL_TARGETS(M) \ M(avx512knl_x16) #define AVX512SKX_TARGETS(M) \ M(avx512skx_x4) \ M(avx512skx_x8) \ M(avx512skx_x16) \ M(avx512skx_x32) \ M(avx512skx_x64) #define AVX512SPR_TARGETS(M) \ M(avx512spr_x4) \ M(avx512spr_x8) \ M(avx512spr_x16) \ M(avx512spr_x32) \ M(avx512spr_x64) #define X86_TARGETS(M) \ SSE2_TARGETS(M) \ SSE4_TARGETS(M) \ AVX1_TARGETS(M) \ AVX2_TARGETS(M) \ AVX512KNL_TARGETS(M) \ AVX512SKX_TARGETS(M) \ AVX512SPR_TARGETS(M) #define TARGET_BITCODE_LIB(TARGET, OS_NAME, OS_TARGET, BIT, ARCH) static BitcodeLib target_ ## TARGET ## _ ## BIT ## bit_ ## OS_NAME("builtins-" #TARGET "-" #BIT "bit-" #OS_NAME ".bc", ISPCTarget::TARGET, TargetOS::OS_TARGET, Arch::ARCH) #define TARGET_BITCODE_LIB_UNIX_X86_64(TARGET) TARGET_BITCODE_LIB(TARGET, unix, linux, 64, x86_64) #define TARGET_BITCODE_LIB_UNIX_X86(TARGET) TARGET_BITCODE_LIB(TARGET, unix, linux, 32, x86) #define TARGET_BITCODE_LIB_WINDOWS_X86_64(TARGET) TARGET_BITCODE_LIB(TARGET, windows, windows, 64, x86_64) #define TARGET_BITCODE_LIB_WINDOWS_X86(TARGET) TARGET_BITCODE_LIB(TARGET, windows, windows, 32, x86) X86_TARGETS(TARGET_BITCODE_LIB_UNIX_X86_64) X86_TARGETS(TARGET_BITCODE_LIB_UNIX_X86) X86_TARGETS(TARGET_BITCODE_LIB_WINDOWS_X86_64) X86_TARGETS(TARGET_BITCODE_LIB_WINDOWS_X86)

It might be worth considering if the future extensions or support of this code would be easier.

Maybe we can use CMake configuration template here?

Maybe we can use CMake configuration template here?

What for?

What for?

To avoid manually maintaining the list of builtins, which is prone to errors. I think it can be more efficient to utilize CMake. CMake is already aware of all target names, architectures, and compilation definitions. It would also help to concentrate all logic regarding build in one place.

To avoid manually maintaining the list of builtins, which is prone to errors.
I doubt that CMake code would be free of it.

I think it can be more efficient to utilize CMake. CMake is already aware of all target names, architectures, and compilation definitions. It would also help to concentrate all logic regarding build in one place.

I disagree that this is build logic.

aneshlya

I'd like to do more thorough review but here are some initial comments.

CMakeLists.txt

src/dist.h

aneshlya · 2024-04-08T06:34:19Z

src/slim.cpp

+
+// Target-specific built-in bitcode_libs
+// clang-format off
+#if defined(ISPC_UNIX_TARGET_ON) && defined(ISPC_X86_ENABLED)


Maybe we can use CMake configuration template here?

This change follows the redesign design document as quoted here: > 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: > > 1. Slim Binary: Distribute them alongside the ISPC binary within a > designated directory structure. > 2. 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: > > 1. Generation of all built-in library bitcode files. > 2. Compilation of the slim ISPC binary. > 3. Generation of standard library bitcode files by the slim ISPC binary. > 4. 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 steps 1, 2, 4 (excluding non-existent yet header) are implemented here. Default build includes the last step and configures to build and install ispc-whole named as ispc. In default configuration, ispc target corresponds to ispc-whole, i.e., behavior is as same as before. Under ISPC_SLIM_BINARY=ON, ispc target corresponds to ispc-slim that does not contain builtin libraries inside binary, they are placed alongside as LLVM bitcode files. New build targets are introduced: - dispatch-builtins-bc, - common-builtins-bc, - target-builtins-bc, - builtins-bc, - dispatch-builtins-cpp, - common-builtins-cpp, - target-builtins-cpp, - builtins-cpp, - ispc-slim. Generation of builtins-*.cpp are splitted to two stages: 1. generation of LLVM bitcode files (to use them for ispc-slim), corresponding targets (dispatch-builtins, common-builtins, target-builtins, builtins), 2. generation of CPP with embedded LLVM bitcode (to use them to compile ispc-whole), corresponding targets (dispatch-builtins-cpp, common-builtins-cpp, target-builtins-cpp, builtins-cpp).

nurmukhametov force-pushed the two-stage-build branch 2 times, most recently from dda5818 to df038af Compare April 1, 2024 17:05

nurmukhametov changed the title ~~WIP: Two stage build~~ Two stage build Apr 1, 2024

nurmukhametov requested review from turinevgeny, aneshlya and dbabokin April 1, 2024 17:12

nurmukhametov force-pushed the two-stage-build branch from df038af to 4bd3e3b Compare April 2, 2024 12:30

turinevgeny reviewed Apr 3, 2024

View reviewed changes

nurmukhametov force-pushed the two-stage-build branch from 4bd3e3b to af239d0 Compare April 3, 2024 13:25

aneshlya reviewed Apr 8, 2024

View reviewed changes

nurmukhametov force-pushed the two-stage-build branch from af239d0 to 804fb23 Compare April 8, 2024 14:49

nurmukhametov force-pushed the two-stage-build branch from 804fb23 to af50f41 Compare May 8, 2024 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Two stage build #2823

Two stage build #2823

nurmukhametov commented Apr 1, 2024 •

edited

turinevgeny Apr 3, 2024

nurmukhametov Apr 3, 2024

turinevgeny Apr 3, 2024

nurmukhametov Apr 3, 2024

turinevgeny Apr 3, 2024

nurmukhametov Apr 3, 2024

turinevgeny Apr 3, 2024

nurmukhametov Apr 3, 2024

turinevgeny Apr 3, 2024

nurmukhametov Apr 3, 2024

turinevgeny Apr 3, 2024

nurmukhametov Apr 3, 2024

turinevgeny Apr 3, 2024

nurmukhametov Apr 3, 2024 •

edited

turinevgeny Apr 3, 2024

aneshlya Apr 8, 2024

nurmukhametov Apr 8, 2024

aneshlya Apr 8, 2024

nurmukhametov Apr 9, 2024

aneshlya left a comment

aneshlya Apr 8, 2024

	# Set hidden visibility for inline functions. This is needed to be in sync
	# Set hidden visibility for inline functions. This is needed to be in sync

	// then change the arch if needed). Somehow that ends up giving us
	// then change the arch if needed). Somehow that ends up giving us

	// 'i386-apple-macosx10.7.0'. And then LLVM issues a warning about
	// 'i386-apple-macosx10.7.0'. And then LLVM issues a warning about

	// TODO! strangly enough this TargetOS has no sense and this dispatch lib is used on windows
	// TODO! strangely enough this TargetOS has no sense and this dispatch lib is used on windows

Two stage build #2823

Are you sure you want to change the base?

Two stage build #2823

Conversation

nurmukhametov commented Apr 1, 2024 • edited

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

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

nurmukhametov Apr 3, 2024 • edited

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

aneshlya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nurmukhametov commented Apr 1, 2024 •

edited

nurmukhametov Apr 3, 2024 •

edited