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.

Sign up for GitHub

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

Jump to bottom

Make sure we don't promise alignments that are larger than the heap alignment to LLVM #56938

Merged
merged 3 commits into from
Jan 3, 2025
Merged

Make sure we don't promise alignments that are larger than the heap alignment to LLVM #56938

merged 3 commits into from
Jan 3, 2025

Conversation

gbaraldi
Copy link
Member

@gbaraldi gbaraldi commented Jan 2, 2025

Fixes #56937

@gbaraldi gbaraldi added compiler:codegen Generation of LLVM IR and native code backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 labels Jan 2, 2025
@gbaraldi gbaraldi requested a review from vtjnash January 2, 2025 19:52
vtjnash
vtjnash reviewed Jan 2, 2025
View reviewed changes
src/codegen.cpp
Comment on lines 8917 to +8920
TypeSize sz = DL.getTypeAllocSize(RT);
Align al = DL.getPrefTypeAlign(RT);
if (al > MAX_ALIGN)
al = Align(MAX_ALIGN);
Copy link
Member

Choose a reason for hiding this comment

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

Could we change these to use dereferenceable_size and julia_alignment functions? I usually distrust all DataLayout-based values, since they based on conversions to llvm types which require accuracy in first-class aggregates in matching C, which isn't entirely accurate

Copy link
Member Author

Choose a reason for hiding this comment

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

I’ll give it a shot

Copy link
Member Author

Choose a reason for hiding this comment

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

There doesn't seem to be a super precise type to get for all arguments sadly.

This was referenced Jan 3, 2025
Open
Closed
@oscardssmith oscardssmith merged commit 1e2758e into master Jan 3, 2025
7 checks passed
@oscardssmith oscardssmith deleted the gb/max-align branch January 3, 2025 17:20
@oscardssmith oscardssmith mentioned this pull request Jan 3, 2025
Merged
This was referenced Jan 28, 2025
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oscardssmith oscardssmith oscardssmith approved these changes

@vtjnash vtjnash vtjnash approved these changes

Assignees
No one assigned
Labels
backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

We lie to LLVM about alignment of VecElement types.
3 participants
@gbaraldi @vtjnash @oscardssmith