Skip to content

Support LLVM 19.1#14842

Merged
straight-shoota merged 6 commits intocrystal-lang:masterfrom
HertzDevil:feature/llvm-19
Sep 29, 2024
Merged

Support LLVM 19.1#14842
straight-shoota merged 6 commits intocrystal-lang:masterfrom
HertzDevil:feature/llvm-19

Conversation

@HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Jul 30, 2024

Closes #14840. Tested on Debian using 1:19.1.0~++20240730073008+404746b9f21b-1~exp1~20240730073118.4.

The return type of LLVM::DIBuilder#insert_declare_at_end now differs between LibLLVM::ValueRef and LibLLVM::DbgRecordRef depending on whether LLVM is new enough (both are typedefs to Void*), although this return value is never used in the compiler itself.

LibLLVM.di_builder_insert_declare_at_end(self, storage, var_info, expr, dl, block)
{% else %}
LibLLVM.di_builder_insert_declare_record_at_end(self, storage, var_info, expr, dl, block)
{% end %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my, we're returning a raw Void* from LLVM here, which may be a LibLLVM::ValueRef or a LibLLVM::DbgRecordRef now 😞

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's only a single call to this method in the compiler, and the return value is ignored. Maybe we should tack on a type restriction to Nil? 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the crystal compiler we may not care, but the method is public...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we do the sensible thing of wrapping those pointers into an actual LLVM::Metadata or LLVM::DebugRecord struct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also LLVM::DIBuilder is marked as @[Experimental], so we are free to make breaking changes here, and probably not in a hurry to do so

@HertzDevil
Copy link
Contributor Author

19.1.0 stable has been released; awaiting the volunteer packages for CI

@HertzDevil
Copy link
Contributor Author

It appears there are now official binaries by release managers and all the third-party ones are only on their forum: https://discourse.llvm.org/t/rfc-improve-binary-security/78121/56

LibLLVM.di_builder_insert_declare_at_end(self, storage, var_info, expr, dl, block)
{% else %}
LibLLVM.di_builder_insert_declare_record_at_end(self, storage, var_info, expr, dl, block)
{% end %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's only a single call to this method in the compiler, and the return value is ignored. Maybe we should tack on a type restriction to Nil? 🤷

@HertzDevil HertzDevil marked this pull request as ready for review September 19, 2024 14:56
@straight-shoota straight-shoota modified the milestones: 1.15.0, 1.14.0 Sep 27, 2024
@straight-shoota
Copy link
Member

I think it's fine to still add this into 1.14.0 even though we're already in the freeze period. The changes only have an effect when linking against LLVM 1.19, so this should not affect any existing configuration.

@straight-shoota straight-shoota merged commit 8692740 into crystal-lang:master Sep 29, 2024
@HertzDevil HertzDevil deleted the feature/llvm-19 branch October 2, 2024 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LLVM 19 support

3 participants