Page MenuHomePhabricator

[GVN] non-functional code movement
ClosedPublic

Authored by vtjnash on Mar 22 2019, 5:00 PM.

Details

Summary

In preparation for later fixes to the non-integral addrspace handling (D59661)

Diff Detail

Repository
rL LLVM

Event Timeline

vtjnash created this revision.Mar 22 2019, 5:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 22 2019, 5:00 PM
reames accepted this revision.Mar 26 2019, 6:02 PM

LGTM w/one required change.

lib/Transforms/Utils/VNCoercion.cpp
314 ↗(On Diff #191976)

Remove this line, not NFC

This revision is now accepted and ready to land.Mar 26 2019, 6:02 PM
vtjnash marked 2 inline comments as done.Mar 26 2019, 6:17 PM
vtjnash added inline comments.
lib/Transforms/Utils/VNCoercion.cpp
314 ↗(On Diff #191976)
331 ↗(On Diff #191976)

OTOH, this might not be strictly NFC, since, while not a semantic difference in IR, I thought it seemed like ConstantFoldLoadFromConstPtr may sometimes have difficultly looking through this BitCast.

vtjnash updated this revision to Diff 196651.Apr 25 2019, 9:18 AM

remove change that could result in a functional difference

@vtjnash Was the review comment addressed? If so I assume you want me to land this, since you don't have commit?

vtjnash marked an inline comment as done.May 7 2019, 9:08 AM
vtjnash added inline comments.
lib/Transforms/Utils/VNCoercion.cpp
314 ↗(On Diff #191976)

Bump. I wanted to confirm you agree now that this part of the change is NFC. I think I've addressed your other comments, so the other commits in the stack should also be ready for re-review also. Thanks!

@loladiro can you land this for me?

loladiro added inline comments.Jun 7 2019, 2:31 PM
lib/Transforms/Utils/VNCoercion.cpp
314 ↗(On Diff #191976)

I've looked at this and I think I concur with this analysis, so I'll go ahead and land this, since @reames was ok with the rest of it. @reames If we both missed something, please do let me know and I'll be happy to revert or adjust.

Closed by commit rL362853: [GVN] non-functional code movement (authored by kfischer, committed by ). · Explain WhyJun 7 2019, 4:08 PM
This revision was automatically updated to reflect the committed changes.