Page MenuHomePhabricator

[BDCE] Skip metadata while replacing uses
ClosedPublic

Authored by davide on Dec 7 2016, 1:01 PM.

Details

Summary

The fix committed in r288851 doesn't cover all the cases. In particular, if we have an instruction with side effects which has a no non-dbg use not depending on the bits, we still perform RAUW destroying the dbg.value's first argument.
Prevent metadata from being replaced here to avoid the issue.

Diff Detail

Repository
rL LLVM

Event Timeline

davide updated this revision to Diff 80643.Dec 7 2016, 1:01 PM
davide retitled this revision from to [BDCE] Skip metadata while replacing uses.
davide updated this object.
davide added a reviewer: hfinkel.
davide added a subscriber: llvm-commits.
majnemer added inline comments.
include/llvm/IR/Value.h
277 ↗(On Diff #80643)

How about replaceNonMetadataUsesWith

lib/Transforms/Scalar/BDCE.cpp
42–45 ↗(On Diff #80643)

I think we can remove this special case now.

davide added inline comments.Dec 7 2016, 1:08 PM
include/llvm/IR/Value.h
277 ↗(On Diff #80643)

Sure.

lib/Transforms/Scalar/BDCE.cpp
42–45 ↗(On Diff #80643)

Hal suggested to keep it as a micro-optimization. I'm fine either way.

davide updated this revision to Diff 80645.Dec 7 2016, 1:11 PM

Renamed an API as suggested by David.

hfinkel accepted this revision.Dec 7 2016, 1:19 PM
hfinkel edited edge metadata.

LGTM

lib/Transforms/Scalar/BDCE.cpp
42–45 ↗(On Diff #80643)

I don't feel strongly about it. It does make sense as a micrp-optimization because it will avoid us computing known bits on an instruction where that will never help us.

This revision is now accepted and ready to land.Dec 7 2016, 1:19 PM
This revision was automatically updated to reflect the committed changes.