Skip to content

Commit 6b38b4e

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
1 parent 3cdeb53 commit 6b38b4e

File tree

2 files changed

+28
-23
lines changed

2 files changed

+28
-23
lines changed

src/CMakeLists.txt

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

553+
set(Halide_COMPILER_BUILTIN_LIBRARY "${Halide_COMPILER_BUILTIN_LIBRARY}"
554+
CACHE FILEPATH "Library containing __udivdi3 symbol, either “libgcc.a” or “libclang_rt.builtins.*.a”.")
555+
556+
target_link_libraries(Halide PRIVATE "${Halide_COMPILER_BUILTIN_LIBRARY}")
557+
553558
# Note that we (deliberately) redeclare these versions here, even though the macros
554559
# with identical versions are expected to be defined in source; this allows us to
555560
# ensure that the versions defined between all build systems are identical.

src/JITModule.cpp

Lines changed: 23 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,25 @@ void load_webgpu() {
135133
<< "(Try setting the env var HL_WEBGPU_NATIVE_LIB to an explicit path to fix this.)\n";
136134
}
137135

136+
static 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+
{"__udivdi3", (void *)&__udivdi3},
143+
};
144+
145+
llvm::orc::SymbolMap AdditionalSymbols;
146+
for (const auto &NamePtr : NamePtrList) {
147+
auto Addr = static_cast<llvm::orc::ExecutorAddr>(
148+
reinterpret_cast<uintptr_t>(NamePtr.second));
149+
AdditionalSymbols[Jit.mangleAndIntern(NamePtr.first)] =
150+
llvm::orc::ExecutorSymbolDef(Addr, llvm::JITSymbolFlags::Exported);
151+
}
152+
return AdditionalSymbols;
153+
}
154+
138155
} // namespace
139156

140157
using namespace llvm;
@@ -216,25 +233,6 @@ class HalideJITMemoryManager : public SectionMemoryManager {
216233
}
217234
}
218235
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
238236
internal_assert(result != 0)
239237
<< "HalideJITMemoryManager: unable to find address for " << name << "\n";
240238
return result;
@@ -350,6 +348,8 @@ void JITModule::compile_module(std::unique_ptr<llvm::Module> m, const string &fu
350348
internal_assert(gen) << llvm::toString(gen.takeError()) << "\n";
351349
JIT->getMainJITDylib().addGenerator(std::move(gen.get()));
352350

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

0 commit comments

Comments
 (0)