This is an archive of the discontinued LLVM Phabricator instance.

[DSE] Add OR_None (not for commit)
AbandonedPublic

Authored by fhahn on Oct 22 2021, 6:13 AM.

Details

Reviewers
None
Summary

Slight variant of D105098 which also returns OW_None on NoAlias.

Diff Detail

Event Timeline

fhahn created this revision.Oct 22 2021, 6:13 AM
fhahn requested review of this revision.Oct 22 2021, 6:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 22 2021, 6:13 AM
nikic added a subscriber: nikic.Oct 23 2021, 2:18 AM
nikic added inline comments.
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
1012

Maybe move this check to the start?

1069

Comment is outdated.

fhahn abandoned this revision.Oct 31 2021, 9:42 AM

This is not really intended for commit, just to make it easy to apply/test the follow-up changes.

Let's work on getting the original D105098 in.

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
1012

I originally had the check just after getting AAR, but isOverwrite considers out-of-bounds writes killing other writes to the same underlying object, which would be missed by the early exit.

ebrevnov added inline comments.
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
1012

Could you clarify which particular piece of code handles "out-of-bounds" case?

fhahn added inline comments.Nov 12 2021, 1:44 AM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
1012

It is not handled explicitly and is a consequence of the code just below (lines 1017-1020). If the underlying object size matches the size of the out-of-bounds store, the out of bounds store is considered killing other stores to the same object. Note that this logic does not kick in, if the out-of-bounds store is smaller than the underlying object. I added such a test case in 9c00afe926e9

ebrevnov added inline comments.Nov 15 2021, 12:59 AM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
1012

Got it, thanks. Will update D105098 and add related comment.

fhahn added inline comments.Nov 15 2021, 3:00 AM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
1012

That sounds great, thanks!

fhahn added inline comments.Nov 19 2021, 3:08 AM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
1012

@ebrevnov do you already have an idea on when you'll be able to update D105098?

otherwise I could also fold the changes from D105098 into this one and commit it on your behalf.

I'm totally fine if you commit changes by yourself. I will be able to get to it early next week only.

PS: I had a very busy week and didn't have chance to work on that. Sorry...