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

alignment: subtly change meaning of datatype_align #34473

Merged
merged 1 commit into from
Jan 30, 2020

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Jan 22, 2020

Rather than meaning the actual alignment of the object, this now means
the preferred alignment of the object. The actual alignment of any
object is the minimum of this preferred alignment and the alignment
supported by the runtime allocator. This aligns us with how LLVM treats
alignment, and is probably reasonably sensible anyways.

Also tries to audit our existing uses of CreateLoad/CreateStore for
correctness, and upgrade some to include pointer-types.

fixes #32414

Copy link
Sponsor Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@JeffBezanson
Copy link
Sponsor Member

Let's rebase this and try to get a better CI run.

v = ntuple(w -> Float64(10w), Val(8))
return a, (v, (a, (1e6, 1e9)))
end
@test bar32414(-35.0) === (-35.0, ((10.0, 20.0, 30.0, 40.0, 50.0, 60.0, 70.0, 80.0), (-35.0, (1.0e6, 1.0e9))))
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Unlike the example in #32414 this test passes for me on master. Should we add a version that includes VecElement?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

oh, I just pasted the wrong one here

Rather than meaning the actual alignment of the object, this now means
the preferred alignment of the object. The actual alignment of any
object is the minimum of this preferred alignment and the alignment
supported by the runtime allocator. This aligns us with how LLVM treats
alignment, and is probably reasonably sensible anyways.

Also tries to audit our existing uses of CreateLoad/CreateStore for
correctness, and upgrade some to include pointer-types.

fixes #32414
@JeffBezanson JeffBezanson merged commit 7426625 into master Jan 30, 2020
@JeffBezanson JeffBezanson deleted the jn/vec-align-32414 branch January 30, 2020 20:49
@KristofferC
Copy link
Sponsor Member

Doesn't backport cleanly. Can push a backport to #34517.

@KristofferC
Copy link
Sponsor Member

Bump for backport.

vtjnash added a commit that referenced this pull request Feb 4, 2020
Rather than meaning the actual alignment of the object, this now means
the preferred alignment of the object. The actual alignment of any
object is the minimum of this preferred alignment and the alignment
supported by the runtime allocator. This aligns us with how LLVM treats
alignment, and is probably reasonably sensible anyways.

Also tries to audit our existing uses of CreateLoad/CreateStore for
correctness, and upgrade some to include pointer-types.

fixes #32414
KristofferC pushed a commit that referenced this pull request Feb 5, 2020
Rather than meaning the actual alignment of the object, this now means
the preferred alignment of the object. The actual alignment of any
object is the minimum of this preferred alignment and the alignment
supported by the runtime allocator. This aligns us with how LLVM treats
alignment, and is probably reasonably sensible anyways.

Also tries to audit our existing uses of CreateLoad/CreateStore for
correctness, and upgrade some to include pointer-types.

fixes #32414

(cherry picked from commit 268ac24)
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
Rather than meaning the actual alignment of the object, this now means
the preferred alignment of the object. The actual alignment of any
object is the minimum of this preferred alignment and the alignment
supported by the runtime allocator. This aligns us with how LLVM treats
alignment, and is probably reasonably sensible anyways.

Also tries to audit our existing uses of CreateLoad/CreateStore for
correctness, and upgrade some to include pointer-types.

fixes #32414
BioTurboNick pushed a commit to BioTurboNick/julia that referenced this pull request Apr 13, 2020
Rather than meaning the actual alignment of the object, this now means
the preferred alignment of the object. The actual alignment of any
object is the minimum of this preferred alignment and the alignment
supported by the runtime allocator. This aligns us with how LLVM treats
alignment, and is probably reasonably sensible anyways.

Also tries to audit our existing uses of CreateLoad/CreateStore for
correctness, and upgrade some to include pointer-types.

fixes JuliaLang#32414

(cherry picked from commit 268ac24)
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.

Wrong GEP emission for tuple containing VecElement
4 participants