-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Remove debug information globals #43770
Conversation
Doesn't this need to get shared among all |
What happens if we create the debug information builders multiple times? Is it just a performance loss, or will LLVM silently break debuginfo? |
It shouldn't horribly fail, since the types don't have to agree, but will probably make unique types that are supposed to be the same. |
79101e1
to
321e677
Compare
src/aotcompile.cpp
Outdated
@@ -453,11 +453,11 @@ void jl_dump_native_impl(void *native_code, | |||
TheTriple.setOS(llvm::Triple::MacOSX); | |||
#endif | |||
std::unique_ptr<TargetMachine> TM( | |||
jl_TargetMachine->getTarget().createTargetMachine( | |||
jl_ExecutionEngine->getTargetMachine().getTarget().createTargetMachine( |
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.
this seems a little bit odd. Should we use a TargetMachineBuilder?
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.
We probably should, especially since we can choose to not expose the actual TargetMachine itself from the JIT engine.
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.
Update on this: my attempt at using a TargetMachineBuilder failed during building on macos, so I'm thinking it's better to revisit rewriting this particular sequence of instructions at a later date.
321e677
to
1c31d84
Compare
1c31d84
to
d34e739
Compare
d34e739
to
3057da1
Compare
Another PR in the line of removing the codegen globals.
Depends on #43802