This is an archive of the discontinued LLVM Phabricator instance.

Make sure we preserve alignment information after hoisting invariant load
ClosedPublic

Authored by hulx2000 on Jan 13 2016, 3:38 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

hulx2000 retitled this revision from to Make sure we preserve alignment information after hoisting invariant load.
hulx2000 updated this object.
hulx2000 added a reviewer: grosser.
hulx2000 set the repository for this revision to rL LLVM.
hulx2000 added a project: Restricted Project.
hulx2000 added a subscriber: llvm-commits.

Suggestion for a small improvement.

lib/CodeGen/IslNodeBuilder.cpp
950

Why do you try to find LoadInst after the call to "preloadUnconditionally" and set the alignment there instead of in the ditonally function when the load is actually created?

I would suggest to change the last argument from AccInstTy to AccInst and set the alignment then inside the function. What do you say?

I agree, that's better, I will update that.

updated to address Johannes's comment, thanks Johannes

hulx2000 marked an inline comment as done.Jan 14 2016, 3:59 PM
jdoerfert accepted this revision.Jan 14 2016, 4:21 PM
jdoerfert edited edge metadata.

LGTM. You can commit this or tell me if I should commit it. Thanks for the update!

This revision is now accepted and ready to land.Jan 14 2016, 4:21 PM
jdoerfert added inline comments.Jan 14 2016, 4:24 PM
lib/CodeGen/IslNodeBuilder.cpp
923

I forgot. We should set the alingment here too. Sorry.

hulx2000 marked an inline comment as done.Jan 14 2016, 4:30 PM
hulx2000 added inline comments.
lib/CodeGen/IslNodeBuilder.cpp
923

Oh, sorry, I missed that one too.

hulx2000 edited edge metadata.
hulx2000 marked an inline comment as done.

updated again

hulx2000 marked an inline comment as done.Jan 14 2016, 4:35 PM
This revision was automatically updated to reflect the committed changes.