I guess this is more generic apporach than https://reviews.llvm.org/D134610.

I think it makes more sense to use stable version of clang-format for that purpose but I'm not in a postion to decide as I don't work with format. Can you change the script to use latest clang-format for format codebase and revert this patch in the meantime? Currently, all patches that touch format part will see format verifiaction ci faliure with this patch.

Wouldn't that mean llvm contributors have to format their code using clang-format built from top-of-tree source code before submitting the patch? Technically that script is for checking if the code has been formatted correctly.

I think buildbot is "formatting" the patch using clang-format-15. The tests of clang-format would be done with latest code.

This seems to be causing builtbot lint script error: https://buildkite.com/llvm-project/premerge-checks/builds/134501#01862630-8fa9-487c-99b1-9addbe6257d0

We don't need this now.

Ping? Also wonder if this is not a problem anymore.

LGTM for me. That validation error has been a problem on Windows too.

Sorry for the delay. I was only looking at the test case; the code change looks reasonable to me. As they are supposed to be discarded anyways, it's safe to just remove COMDAT data as well to make IR valid.

What happens when we actually have duplicate comdat any definitions? IIRC I've seen that in msvcrt.lib.

Maybe @Hahnfeld knows something as he added riscv support to clang-repl recently.

Do you have an exception support in your libstdc++ and it's dynamically linked? I believe if clang-repl was able to execute the program, it must have successfully linked to cxxthrow symbols and so exception handling works correctly.

I think you could just do default: break; but logging can be helpful at this stage of development I guess.

LGTM to me in general. Few minor comments.

LGTM!

Fix test failure. Separate out the platform instance setup part in order to load orc runtime when JIT instance is fully configured. (e.g. in-process symbol generator is set up)

This is actually not related to the conditionals but related to something in my system or cmake which causes it to set MINGW to true. Abandoning this patch as it is going to break clang-cl building case anyways.

Keep only the part relevant for MSVC.

Turned out the different library name was made by a bug in compiler-rt cmake file -- my local build was generating ".a" library files. It should be fixed in https://reviews.llvm.org/D133669. I'll update the code to use just ".lib" I'll invastigate the symbol not found failure shortly.

I mistakenly commited this change to irrelevant commit (https://github.com/llvm/llvm-project/commit/d1c4d96126921e02a8f39b9a8ffa3c875158ffaf)

In D130479#3776221, @yingcong-wu wrote:

check-all failed with those tests:

ORC-x86_64-windows :: TestCases/Windows/x86-64/hello-world.c
ORC-x86_64-windows :: TestCases/Windows/x86-64/hello-world.cpp
ORC-x86_64-windows :: TestCases/Windows/x86-64/priority-static-initializer-three.c
ORC-x86_64-windows :: TestCases/Windows/x86-64/priority-static-initializer.c
ORC-x86_64-windows :: TestCases/Windows/x86-64/sehframe-handling.cpp
ORC-x86_64-windows :: TestCases/Windows/x86-64/static-initializer.S
ORC-x86_64-windows :: TestCases/Windows/x86-64/static-initializer.cpp

Directly caused by incorrect orc runtime library filename in Windows.

But after changing the filename in compiler-rt/test/orc/lit.cfg.py, tests reported

llvm-jitlink error: Could not find symbol at given index, did you add it to JITSymbolTable? index: 441, section: 3001
D:\bin\llvm-jitlink.exe: Failed to materialize symbols: { (main, { __orc_rt_coff_symbol_lookup_tag }) }

Build Info

• Build type: Debug
• Target: X86
• In tree build

Could you please give me some clues?

Add & fix tests.

In D130788#3732325, @sbc100 wrote:

In D130788#3731232, @sunho wrote:

In D130788#3730533, @sbc100 wrote:

I'm not totally sure but I think the change is responsible for the emscripten integration bot failing InterpreterTest.CatchException: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8807160007692150337/+/u/LLVM_regression/stdout

[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
JIT session error: Symbols not found: [ __gxx_personality_v0, _ZSt9terminatev, _ZTVN10__cxxabiv117__class_type_infoE, __cxa_allocate_exception, __cxa_begin_catch, __cxa_end_catch, __cxa_free_exception, __cxa_throw ]
Failure value returned from cantFail wrapped call
Failed to materialize symbols: { (main, { _ZN16custom_exceptionC2EPKc, __clang_call_terminate, _ZTI16custom_exception, _ZTS16custom_exception, throw_exception }) }
UNREACHABLE executed at /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/llvm/include/llvm/Support/Error.h:786!
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x2464413)[0x55cb14660413]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246236c)[0x55cb1465e36c]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x24648df)[0x55cb146608df]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x12980)[0x7fad2fab1980]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7fad2eb0de87]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7fad2eb0f7f1]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x23f798f)[0x55cb145f398f]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9de35)[0x55cb14099e35]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9d597)[0x55cb14099597]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246d6be)[0x55cb146696be]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246e659)[0x55cb1466a659]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246ee40)[0x55cb1466ae40]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x247b2c3)[0x55cb146772c3]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x247ab42)[0x55cb14676b42]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246559c)[0x55cb1466159c]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7fad2eaf0c87]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9ceda)[0x55cb14098eda]

This started happening consistently after this change https://chromium.googlesource.com/emscripten-releases/+/584b2f531314d1e70cd5ebadcce7e015a6215c9a. The only CL in that list that looks related seems to be this one.

Could you share the detailed build configuration of those bots?

All the step should be visible here: https://ci.chromium.org/ui/p/emscripten-releases/builders/ci/linux-test-suites/b8805531315340503553/steps

The most relevant one I guess would be:

https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8805531315340503553/+/u/Build_LLVM/stdout

Here you can see LLVM being configured with:

subprocess.check_call(`/b/s/w/ir/cache/builder/emscripten-releases/cmake-3.21.3-linux-x86_64/bin/cmake -G Ninja -DPython3_EXECUTABLE=/b/s/w/ir/cache/vpython/68bda9/bin/python -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/b/s/w/ir/x/w/install -DCMAKE_CXX_FLAGS=-stdlib++-isystem/b/s/w/ir/x/w/install/include/c++/v1 "-DCMAKE_EXE_LINKER_FLAGS=-L/b/s/w/ir/x/w/install/lib -stdlib=libc++" "-DCMAKE_SHARED_LINKER_FLAGS=-L/b/s/w/ir/x/w/install/lib -stdlib=libc++" "-DCMAKE_MODULE_LINKER_FLAGS=-L/b/s/w/ir/x/w/install/lib -stdlib=libc++" -DCMAKE_SYSROOT=/b/s/w/ir/cache/builder/emscripten-releases/sysroot_debian_stretch_amd64 -DCMAKE_C_COMPILER=/b/s/w/ir/cache/builder/v8/third_party/llvm-build/Release+Asserts/bin/clang -DCMAKE_CXX_COMPILER=/b/s/w/ir/cache/builder/v8/third_party/llvm-build/Release+Asserts/bin/clang++ -DCMAKE_C_COMPILER_LAUNCHER=/b/s/w/ir/cache/goma/client/gomacc -DCMAKE_CXX_COMPILER_LAUNCHER=/b/s/w/ir/cache/goma/client/gomacc /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/llvm -DLLVM_ENABLE_LIBXML2=OFF -DLLVM_INCLUDE_EXAMPLES=OFF -DLLVM_BUILD_LLVM_DYLIB=OFF -DLLVM_LINK_LLVM_DYLIB=OFF -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON -DLLVM_ENABLE_BINDINGS=OFF -DLLVM_TOOL_LTO_BUILD=OFF -DLLVM_INSTALL_TOOLCHAIN_ONLY=ON -DLLVM_TARGETS_TO_BUILD=host;WebAssembly -DLLVM_ENABLE_PROJECTS=lld;clang -DLLVM_ENABLE_TERMINFO=0 -DLLVM_USE_CRT_RELEASE=MT -DLLVM_USE_CRT_DEBUG=MTd -DCLANG_ENABLE_ARCMT=OFF -DCLANG_ENABLE_STATIC_ANALYZER=OFF -DCLANG_REPOSITORY_STRING=https://github.com/llvm/llvm-project -DLLVM_ENABLE_LLD=ON -DLLVM_ENABLE_ASSERTIONS=ON`, cwd=`/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out`)

The code the drives the build is here: https://chromium.googlesource.com/emscripten-releases/+/12ac1c00143f5a8a6722bdb8cca9e4515c2654d0/src/build.py#690

Note that we do use out own libc++ build (which is static I believe) but we don't set LLVM_STATIC_LINK_CXX_STDLIB.. we inject it via generic cmake flags: -DCMAKE_CXX_FLAGS=-stdlib++-isystem=.. -DCMAKE_EXE_LINKER_FLAGS=-L... -stdlib=libc++ (this works for the other cmake projects that we also build, as well as llvm). Perhaps we should use be passing LLVM_STATIC_LINK_CXX_STDLIB.

In D130788#3730533, @sbc100 wrote:

I'm not totally sure but I think the change is responsible for the emscripten integration bot failing InterpreterTest.CatchException: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8807160007692150337/+/u/LLVM_regression/stdout

[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from InterpreterTest
[ RUN      ] InterpreterTest.CatchException
JIT session error: Symbols not found: [ __gxx_personality_v0, _ZSt9terminatev, _ZTVN10__cxxabiv117__class_type_infoE, __cxa_allocate_exception, __cxa_begin_catch, __cxa_end_catch, __cxa_free_exception, __cxa_throw ]
Failure value returned from cantFail wrapped call
Failed to materialize symbols: { (main, { _ZN16custom_exceptionC2EPKc, __clang_call_terminate, _ZTI16custom_exception, _ZTS16custom_exception, throw_exception }) }
UNREACHABLE executed at /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/llvm/include/llvm/Support/Error.h:786!
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x2464413)[0x55cb14660413]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246236c)[0x55cb1465e36c]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x24648df)[0x55cb146608df]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x12980)[0x7fad2fab1980]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7fad2eb0de87]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7fad2eb0f7f1]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x23f798f)[0x55cb145f398f]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9de35)[0x55cb14099e35]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9d597)[0x55cb14099597]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246d6be)[0x55cb146696be]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246e659)[0x55cb1466a659]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246ee40)[0x55cb1466ae40]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x247b2c3)[0x55cb146772c3]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x247ab42)[0x55cb14676b42]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246559c)[0x55cb1466159c]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7fad2eaf0c87]
/b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9ceda)[0x55cb14098eda]

This started happening consistently after this change https://chromium.googlesource.com/emscripten-releases/+/584b2f531314d1e70cd5ebadcce7e015a6215c9a. The only CL in that list that looks related seems to be this one.

@kuhar Commited a fix. Thanks for reporting this!

In D130479#3722186, @glandium wrote:

For what it's worth, this break building compiler-rt with clang-cl:

In file included from /builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp:16:
In file included from /builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.h:17:
In file included from /builds/worker/fetches/llvm-project/compiler-rt/lib/orc/executor_address.h:20:
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/simple_packed_serialization.h(403,58): error: no member named 'string_view' 
in namespace 'std'
template <> class SPSSerializationTraits<SPSString, std::string_view> {
                                                    ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(109,21): error: no type named 'string_view' in namespace '
std'
  void *dlopen(std::string_view Name, int Mode);
               ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(111,34): error: no type named 'string_view' in namespace '
std'
  void *dlsym(void *Header, std::string_view Symbol);
                            ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(118,34): error: no member named 'string_view' in namespace
 'std'
      std::vector<std::pair<std::string_view, ExecutorAddrRange>> Secs,
                            ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(122,34): error: no member named 'string_view' in namespace
 'std'
      std::vector<std::pair<std::string_view, ExecutorAddrRange>> Secs);
                            ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(135,36): error: no type named 'string_view' in namespace '
std'
  Expected<void *> dlopenImpl(std::string_view Path, int Mode);
                              ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(143,46): error: no type named 'string_view' in namespace '
std'
  JITDylibState *getJITDylibStateByName(std::string_view Path);
                                        ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(145,54): error: no type named 'string_view' in namespace '
std'
                                                std::string_view Symbol);
                                                ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(156,27): error: no member named 'string_view' in namespace
 'std'
  std::unordered_map<std::string_view, void *> JDNameToHeader;
                     ~~~~~^
/builds/worker/fetches/llvm-project/compiler-rt/lib/orc/coff_platform.cpp(173,55): error: no type named 'string_view' in namespace '
std'

etc.

The problem being that MSVC string_view header doesn't do anything unless building at least in C++17 mode, and for some reason, that's not the case. (In case it matters, this is from a standalone build of compiler-rt)

In D130479#3722135, @mstorsjo wrote:

FYI, this commit broke cross compilation with mingw, due to using the mixed-case Windows.h header, but I fixed that in 156136502b73a53192f21fc0477263e76a17a9b9.

However after that, this new file still produces a bunch of warnings:

[1/2] Building CXX object lib/orc/CMak...RTOrc.x86_64.dir/coff_platform.cpp.obj
In file included from ../lib/orc/coff_platform.cpp:20:
../lib/orc/wrapper_function_utils.h:299:27: warning: address of '__orc_rt_jit_dispatch_ctx' will always evaluate to 'true' [-Wpointer-bool-conversion]
    if (ORC_RT_UNLIKELY(!&__orc_rt_jit_dispatch_ctx))
                        ~ ^~~~~~~~~~~~~~~~~~~~~~~~~
../lib/orc/compiler.h:60:55: note: expanded from macro 'ORC_RT_UNLIKELY'
#define ORC_RT_UNLIKELY(EXPR) __builtin_expect((bool)(EXPR), false)
                                                      ^~~~
../lib/orc/coff_platform.cpp:506:9: warning: unused variable 'JDState' [-Wunused-variable]
  auto &JDState = I->second;
        ^
../lib/orc/coff_platform.cpp:671:44: warning: multi-character character constant [-Wmultichar]
  constexpr uint32_t EH_EXCEPTION_NUMBER = 'msc' | 0xE0000000;
                                           ^
In file included from ../lib/orc/coff_platform.cpp:20: 
../lib/orc/wrapper_function_utils.h:299:27: warning: address of '__orc_rt_jit_dispatch_ctx' will always evaluate to 'true' [-Wpointer-bool-conversion]
    if (ORC_RT_UNLIKELY(!&__orc_rt_jit_dispatch_ctx)) 
                        ~ ^~~~~~~~~~~~~~~~~~~~~~~~~
../lib/orc/compiler.h:60:55: note: expanded from macro 'ORC_RT_UNLIKELY'
#define ORC_RT_UNLIKELY(EXPR) __builtin_expect((bool)(EXPR), false)
                                                      ^~~~
../lib/orc/coff_platform.cpp:314:30: note: in instantiation of function template specialization '__orc_rt::WrapperFunction<__orc_rt::SPSExpected<__orc_rt::SPSSequence<__orc_rt::SPSTuple<__orc_rt::SPSExecutorAddr, __orc_rt::SPSSequence<__orc_rt::SPSExecutorAddr>>>> (__orc_rt::SPSExecutorAddr)>::call<__orc_rt::Expected<std::unordered_map<__orc_rt::ExecutorAddr, std::vector<__orc_rt::ExecutorAddr>>>, __orc_rt::ExecutorAddr>' requested here
          SPSExecutorAddr)>::call(&__orc_rt_coff_push_initializers_tag,
                             ^
In file included from ../lib/orc/coff_platform.cpp:20:
../lib/orc/wrapper_function_utils.h:299:27: warning: address of '__orc_rt_jit_dispatch_ctx' will always evaluate to 'true' [-Wpointer-bool-conversion]
    if (ORC_RT_UNLIKELY(!&__orc_rt_jit_dispatch_ctx))
                        ~ ^~~~~~~~~~~~~~~~~~~~~~~~~
../lib/orc/compiler.h:60:55: note: expanded from macro 'ORC_RT_UNLIKELY'
#define ORC_RT_UNLIKELY(EXPR) __builtin_expect((bool)(EXPR), false) 
                                                      ^~~~
../lib/orc/coff_platform.cpp:441:41: note: in instantiation of function template specialization '__orc_rt::WrapperFunction<__orc_rt::SPSExpected<__orc_rt::SPSExecutorAddr> (__orc_rt::SPSExecutorAddr, __orc_rt::SPSSequence<char>)>::call<__orc_rt::Expected<__orc_rt::ExecutorAddr>, __orc_rt::ExecutorAddr, std::string_view>' requested here
          SPSExecutorAddr, SPSString)>::call(&__orc_rt_coff_symbol_lookup_tag,
                                        ^ 
5 warnings generated.

It'd be nice if this could build without warnings.

Use path as key.

Thanks for the patch! It's really exciting to see i386 backend coming up.

Fixed by sgraenitz using proper way!

Proper support of multiple jitdylibs.

Make COFFPlatform responsible for vc runtime setup.

Change API

Add more tests.

rebase.

EPC

Add license header.