-
-
Notifications
You must be signed in to change notification settings - Fork 90
feat: aot compilation into executable #331
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very exciting! This is so cool to see coming together, Saket. I think it needs cleanup, but I'm cool with the overall structure of this as a first pass. I know we talked about getting something working and merged first and then building on it, so we're right on track for that.
The key thing we're missing is tests, which will need to go in test/bash/aot
. We can chat this week about how that test suite works and the types of things we'll want to test for. No need to wait until Thursday, if you want to meet sooner.
18292b9
to
81e9699
Compare
All tasks done. |
`compile-module`
No, I'm not using CXX yet. Just using `clang++` doesn't work.
I found `llvm::sys::Program::FindProgramByName("clang++")`. I'll see if
this works.
…On Mon, 19 May, 2025, 12:09 am Jeaye Wilkerson, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In compiler+runtime/src/cpp/jank/aot.cpp
<#331 (comment)>:
> +
+ llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> DiagID(new clang::DiagnosticIDs());
+
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = llvm::vfs::getRealFileSystem();
+
+ clang::DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagClient, /*ShouldOwnClient=*/true);
+
+
+ std::string const TargetTripleStr = llvm::sys::getDefaultTargetTriple();
+ llvm::errs() << "Using target triple: " << TargetTripleStr << "\n";
+
+ /* The Driver needs the path to the executable (used for finding related tools/resources)
+ * For this example, we'll use argv[0] or a placeholder. This might need adjustment
+ * depending on how/where your program is run relative to clang resources.
+ * TODO: infer clang++ path. Will be OS dependent and will require to infer correct version's path */
+ auto const ClangExecutablePath{ "/usr/bin/clang++" };
Just using clang++ doesn't work. I'm not sure if this way of doing
compilation looks for the clang++ binary. Instead, it expects the
executable as the first arg.
The error I got was: clang++: command not found
Is clang++ on your PATH? i.e. can you type it into a shell? Or do you
only put it into CXX? We can look at CXX, but we can't rely on it, since
an AOT compiled jank program shipped to some non-devs machine will not
likely have CXX set. We need them to have clang installed, though, and we
need to be able to find it. Relying on PATH is the most portable way, but
we may also need more beyond that, such as looking into common places if we
don't find it in CXX or PATH.
—
Reply to this email directly, view it on GitHub
<#331 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADXTLBKVRO2R4NFJNLJNE6T27DHWLAVCNFSM6AAAAAB43WC34CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNBZGA3DEMZWG4>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
Sounds like Clang isn't searching for the binary. It expects an absolute path. Using |
c5b698e
to
c8e8fcd
Compare
a257b99
to
8c68640
Compare
diags, | ||
"jank_aot_compilation", | ||
vfs); | ||
driver.setCheckInputsExist(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reasoning for this being false?
When do we use -DVARS vs cli args?
If we want to use CLI args, this will be passed again as a parameter to
`jank_init`. It already has 4 args as of now. Increasing number of args
didn't feel like a good idea for an API fn.
WDYT?
…On Tue, 10 Jun, 2025, 12:22 am Jeaye Wilkerson, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In compiler+runtime/src/cpp/main.cpp
<#331 (comment)>:
> - if(opts.gc_incremental)
- {
- GC_enable_incremental();
- }
This is a flag for the *user* to set when invoking jank. This needs to
remain a flag.
I don't understand why this can't still be in opts.
—
Reply to this email directly, view it on GitHub
<#331 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADXTLBMIEQTEINIGAYCDAZD3CXJYDAVCNFSM6AAAAAB43WC34CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSMJQHE4TEOBYGI>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
When you install jank on your machine, you need to be able to customize, with each invocation, if you want incremental GC or not. Defines happen at compile time. If we use a define, you can't choose, since the jank binary you install will only have the define it was compiled with. You wouldn't use a
I don't get it, dude. We have |
Requires
clojure.core
to be compiled into the project'starget
directory for now.