Skip to content

Commit f634312

Browse files
committed
JITModule: rework/fix __udivdi3 handling
The original workaround is very partial, and was not really working in my experience, even after making it non-GCC specific. Instead: 1. Ensure that the library that actually provides that symbol (as per the compiler used!) is actually linked into. This was not enough still. 2. Replace `HalideJITMemoryManager` hack with a more direct approach of actually telling the JIT the address of the symbol. 3. While there, move the symbol's forward definition to outside of namespaces. It's a global symbol, it makes sense to place it there. This makes python binding tests pass on i386, and i'm really happy about that. Refs. llvm/llvm-project#61289 Inspired by root-project/root#13286 Forwarded: #8389 Gbp-Pq: Name 0010-JITModule-rework-fix-__udivdi3-handling.patch
1 parent 3cdeb53 commit f634312

File tree

2 files changed

+52
-23
lines changed

2 files changed

+52
-23
lines changed

src/CMakeLists.txt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,34 @@ if (WITH_SERIALIZATION_JIT_ROUNDTRIP_TESTING)
550550
target_compile_definitions(Halide PRIVATE WITH_SERIALIZATION_JIT_ROUNDTRIP_TESTING)
551551
endif ()
552552

553+
if (NOT DEFINED Halide_COMPILER_BUILTIN_LIBRARY)
554+
# FIXME: `execute_process()` can not use `$<TARGET_FILE:clang>`, workarounding that.
555+
get_property(clang_LOCATION TARGET clang PROPERTY LOCATION)
556+
execute_process(
557+
COMMAND ${CMAKE_CROSSCOMPILING_EMULATOR} ${CMAKE_C_COMPILER_LAUNCHER} ${clang_LOCATION} "--print-libgcc-file-name"
558+
OUTPUT_VARIABLE Halide_COMPILER_BUILTIN_LIBRARY
559+
OUTPUT_STRIP_TRAILING_WHITESPACE
560+
ERROR_VARIABLE Halide_COMPILER_BUILTIN_LIBRARY_stderr
561+
ERROR_STRIP_TRAILING_WHITESPACE
562+
RESULT_VARIABLE Halide_COMPILER_BUILTIN_LIBRARY_exit_code
563+
)
564+
if (NOT "${Halide_COMPILER_BUILTIN_LIBRARY_exit_code}" STREQUAL "0")
565+
message(FATAL_ERROR "Unable to invoke clang --print-libgcc-file-name: ${Halide_COMPILER_BUILTIN_LIBRARY_stderr}")
566+
endif()
567+
endif ()
568+
569+
set(Halide_COMPILER_BUILTIN_LIBRARY "${Halide_COMPILER_BUILTIN_LIBRARY}"
570+
CACHE FILEPATH "Library containing __udivdi3 symbol, either “libgcc.a” or “libclang_rt.builtins.*.a”.")
571+
572+
add_library(Halide::__udivdi3 UNKNOWN IMPORTED)
573+
set_target_properties(
574+
Halide::__udivdi3
575+
PROPERTIES
576+
IMPORTED_LOCATION "${Halide_COMPILER_BUILTIN_LIBRARY}"
577+
)
578+
579+
target_link_libraries(Halide PRIVATE Halide::__udivdi3)
580+
553581
# Note that we (deliberately) redeclare these versions here, even though the macros
554582
# with identical versions are expected to be defined in source; this allows us to
555583
# ensure that the versions defined between all build systems are identical.

src/JITModule.cpp

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,13 @@
2323
#include "Pipeline.h"
2424
#include "WasmExecutor.h"
2525

26+
extern "C" unsigned long __udivdi3(unsigned long a, unsigned long b);
27+
2628
namespace Halide {
2729
namespace Internal {
2830

2931
using std::string;
3032

31-
#if defined(__GNUC__) && defined(__i386__)
32-
extern "C" unsigned long __udivdi3(unsigned long a, unsigned long b);
33-
#endif
34-
3533
#ifdef _WIN32
3634
void *get_symbol_address(const char *s) {
3735
return (void *)GetProcAddress(GetModuleHandle(nullptr), s);
@@ -135,6 +133,26 @@ void load_webgpu() {
135133
<< "(Try setting the env var HL_WEBGPU_NATIVE_LIB to an explicit path to fix this.)\n";
136134
}
137135

136+
llvm::orc::SymbolMap GetListOfAdditionalSymbols(const llvm::orc::LLJIT &Jit) {
137+
// Inject a number of symbols that may be in libgcc.a where they are
138+
// not found automatically. See also the upstream issue:
139+
// https://github.com/llvm/llvm-project/issues/61289.
140+
141+
static const std::pair<const char *, const void *> NamePtrList[] = {
142+
// NOTE: at least libgcc does not provide this symbol on 64-bit x86_64, only on 32-bit i386.
143+
{"__udivdi3", (((CHAR_BIT * sizeof(void *)) == 32) ? ((void *)&__udivdi3) : nullptr)},
144+
};
145+
146+
llvm::orc::SymbolMap AdditionalSymbols;
147+
for (const auto &NamePtr : NamePtrList) {
148+
auto Addr = static_cast<llvm::orc::ExecutorAddr>(
149+
reinterpret_cast<uintptr_t>(NamePtr.second));
150+
AdditionalSymbols[Jit.mangleAndIntern(NamePtr.first)] =
151+
llvm::orc::ExecutorSymbolDef(Addr, llvm::JITSymbolFlags::Exported);
152+
}
153+
return AdditionalSymbols;
154+
}
155+
138156
} // namespace
139157

140158
using namespace llvm;
@@ -216,25 +234,6 @@ class HalideJITMemoryManager : public SectionMemoryManager {
216234
}
217235
}
218236
uint64_t result = SectionMemoryManager::getSymbolAddress(name);
219-
#if defined(__GNUC__) && defined(__i386__)
220-
// This is a workaround for an odd corner case (cross-compiling + testing
221-
// Python bindings x86-32 on an x86-64 system): __udivdi3 is a helper function
222-
// that GCC uses to do u64/u64 division on 32-bit systems; it's usually included
223-
// by the linker on these systems as needed. When we JIT, LLVM will include references
224-
// to this call; MCJIT fixes up these references by doing (roughly) dlopen(NULL)
225-
// to look up the symbol. For normal JIT tests, this works fine, as dlopen(NULL)
226-
// finds the test executable, which has the right lookups to locate it inside libHalide.so.
227-
// If, however, we are running a JIT-via-Python test, dlopen(NULL) returns the
228-
// CPython executable... which apparently *doesn't* include this as an exported
229-
// function, so the lookup fails and crashiness ensues. So our workaround here is
230-
// a bit icky, but expedient: check for this name if we can't find it elsewhere,
231-
// and if so, return the one we know should be present. (Obviously, if other runtime
232-
// helper functions of this sort crop up in the future, this should be expanded
233-
// into a "builtins map".)
234-
if (result == 0 && name == "__udivdi3") {
235-
result = (uint64_t)&__udivdi3;
236-
}
237-
#endif
238237
internal_assert(result != 0)
239238
<< "HalideJITMemoryManager: unable to find address for " << name << "\n";
240239
return result;
@@ -350,6 +349,8 @@ void JITModule::compile_module(std::unique_ptr<llvm::Module> m, const string &fu
350349
internal_assert(gen) << llvm::toString(gen.takeError()) << "\n";
351350
JIT->getMainJITDylib().addGenerator(std::move(gen.get()));
352351

352+
cantFail(JIT->getMainJITDylib().define(absoluteSymbols(GetListOfAdditionalSymbols(*JIT))));
353+
353354
llvm::orc::ThreadSafeModule tsm(std::move(m), std::move(jit_module->context));
354355
auto err = JIT->addIRModule(std::move(tsm));
355356
internal_assert(!err) << llvm::toString(std::move(err)) << "\n";

0 commit comments

Comments
 (0)