Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime/jit-rt: revive and migrates the whole thing to OrcJIT v2 #4774

Merged
merged 28 commits into from
Jan 12, 2025

Conversation

liushuyu
Copy link
Contributor

@liushuyu liushuyu commented Nov 7, 2024

This pull request migrates the dynamic compile system to LLVM OrcJIT v2 (the newer "v2" version from post-LLVM 16 with opaque pointers, not the older "v2" version from the LLVM 12 ~ 15 era).

The implementation is currently in very rough shape, and I am not entirely sure what I am doing. However, the "Hello World" example in the documentation has started to work now.

@kinke
Copy link
Member

kinke commented Nov 7, 2024

Hehe, looks like one's motivated, nice! - In case you haven't seem them, there are quite a few lit-tests for this feature: https://github.com/ldc-developers/ldc/tree/master/tests/dynamiccompile. No idea if anything can still be salvaged from #3184, Ivan's WIP from 5 (!) years ago by now.

@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 7, 2024

Hehe, looks like one's motivated, nice! - In case you haven't seem them, there are quite a few lit-tests for this feature: https://github.com/ldc-developers/ldc/tree/master/tests/dynamiccompile. No idea if anything can still be salvaged from #3184, Ivan's WIP from 5 (!) years ago by now.

I see. I did not know there was another pull request doing the same thing from 5 years ago. Now that I see they made very similar changes before, I realize I duplicated a fair amount of changes (but this seems like a form of cross-validation? Two independent implementations look very similar).

@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 9, 2024

there are quite a few lit-tests for this feature: https://github.com/ldc-developers/ldc/tree/master/tests/dynamiccompile.

Now, 100% of those LIT tests are passing (with LLVM 18).

@liushuyu liushuyu force-pushed the orcjit-v2 branch 4 times, most recently from 2c90767 to bd2c4b5 Compare November 9, 2024 19:28
@kinke
Copy link
Member

kinke commented Nov 10, 2024

Very nice progress! - I've pushed 2 commits to reduce the remaining failures on aarch64.

@liushuyu liushuyu force-pushed the orcjit-v2 branch 2 times, most recently from 465a543 to 8cbe0d1 Compare November 11, 2024 18:15
@kinke
Copy link
Member

kinke commented Nov 11, 2024

In my Windows VM, the tests don't hang as they do for CI (with a pushed Win32 fix wrt. __chkstk, now on 32-bit Windows too). Instead, some dynamic-compile tests pass (on both 32 and 64-bit Windows), others fail with errors like:

  • dynamiccompile/simd_simple_opt.d:
    • 64-bit:
      Dynamic compiler fatal: Symbol not found in jitted code: "�.jit_bind" (".jit_bind")
      
    • 32-bit:
      Incorrect number of arguments passed to called function!
        call x86_stdcallcc void @"\01__D3ldc15dynamic_compile__T4bindTPFG16hZG32hTxG16hZQyFQvxQlZ7wrapperFSQCpQCo__TQCaTQByTxQBqZQCnFQClxQCcZ7ContextQCzZQCy"(ptr inreg noalias align 1 %3, ptr byval(%"ldc.dynamic_compile.bind!(ubyte[32] function(ubyte[16]), const(ubyte[16])).bind.Context") align 4 %4) #0
      LLVM ERROR: Broken module found, compilation aborted!
      
  • dynamiccompile/simple.d:
    • 64-bit:
      Dynamic compiler fatal: Symbol not found in jitted code: "�_D6simple3bazFZv" ("_D6simple3bazFZv")
      
    • 32-bit:
      Dynamic compiler fatal: Symbol not found in jitted code: "�__D6simple3barFZi" ("__D6simple3barFZi")
      

Looks like there's some trouble with the \1 () mangle prefix on Windows, which is used to make LLVM not apply any extra mangling (e.g., extra leading underscore on Win32, and vectorcall/stdcall suffix on Win64).

Edit: We do use \1.jit_bind in general for all targets (https://github.com/liushuyu/ldc/blob/9531ab3828e6b4d1fe675cfcc15085d4ce948155/runtime/jit-rt/cpp-so/bind.cpp#L47-L48), but it's only a problem on Windows. So maybe not name/mangle-related after all, but some other Windows-specific issue.

@liushuyu
Copy link
Contributor Author

liushuyu commented Nov 11, 2024

dynamiccompile/simple.d:

  • 64-bit:
    Dynamic compiler fatal: Symbol not found in jitted code: "�_D6simple3bazFZv" ("_D6simple3bazFZv")
    

I have tried to debug this problem on a Windows 11 installation with Visual Studio 2022 (Community Edition).
It seems like either our dynamic compile mechanism setup LLVM in the wrong way, or there is an LLVM bug on Windows: the symbols were not materialized when they should be (LLVM on Unix platforms will materialize the JIT symbols during the lookup, i.e. lazily compile/resolve the symbols).

@liushuyu liushuyu force-pushed the orcjit-v2 branch 2 times, most recently from 60fedcc to 36bd753 Compare November 12, 2024 02:05
@liushuyu
Copy link
Contributor Author

On Ubuntu 20.04 aarch64:

********************
Unexpectedly Passed Tests (1):
  LDC :: dynamiccompile/throw.d

But it's still broken on macOS aarch64 unfortunately

@liushuyu
Copy link
Contributor Author

It seems like I may have discovered several bugs in LLVM that are exclusive to Windows. I have currently worked around them because I am not entirely sure if they are "bugs" or "intended behaviors".

@liushuyu liushuyu force-pushed the orcjit-v2 branch 5 times, most recently from b0606f7 to 045bf6a Compare November 17, 2024 19:51
@liushuyu liushuyu force-pushed the orcjit-v2 branch 2 times, most recently from 3a0b32b to 236a8d9 Compare November 27, 2024 20:28
@liushuyu liushuyu force-pushed the orcjit-v2 branch 2 times, most recently from 3f8bcdf to b32bb4d Compare December 21, 2024 20:44
@liushuyu
Copy link
Contributor Author

liushuyu commented Jan 8, 2025

The CI failures are due to network issues; a retry should fix them.

@thewilsonator
Copy link
Contributor

retry should fix them.

(restarted)

@liushuyu
Copy link
Contributor Author

liushuyu commented Jan 8, 2025

retry should fix them.

(restarted)

Thanks! All CI tests passed now

@thewilsonator
Copy link
Contributor

Did you resolve #4774 (comment) ?

@kinke
Copy link
Member

kinke commented Jan 10, 2025

I've pushed a commit merging the two very similar optimizer.cpp versions; a mechanical refactoring without any intended functional changes. There's probably more room for follow-up refactorings, such as moving setRtCompileVar() to another jitrt-specific module.

@liushuyu
Copy link
Contributor Author

I've pushed a commit merging the two very similar optimizer.cpp versions; a mechanical refactoring without any intended functional changes. There's probably more room for follow-up refactorings, such as moving setRtCompileVar() to another jitrt-specific module.

Thanks!

Hopefully, by switching to the more stable easy LLJIT API, we can last longer before another need for refactoring comes.

@thewilsonator
Copy link
Contributor

Anything else to be done here?

@kinke kinke enabled auto-merge January 12, 2025 12:36
@kinke kinke merged commit 268ad13 into ldc-developers:master Jan 12, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants