This is an archive of the discontinued LLVM Phabricator instance.

[mssa] If there is no definition in a block prior to an inserted use, return nullptr.
ClosedPublic

Authored by asbirlea on Jun 6 2017, 12:01 PM.

Details

Summary

Check that the first access before one being tested is valid.
Before this patch, if there was no definition prior to the Use being tested,
the first time Iter was deferenced, it hit the sentinel.
Patch updates the check and return nullptr.

Event Timeline

asbirlea created this revision.Jun 6 2017, 12:01 PM
asbirlea edited reviewers, added: george.burgess.iv; removed: gbiv.Jun 6 2017, 12:03 PM
davide added a subscriber: davide.Jun 6 2017, 12:10 PM

Can you write an updater unittest for this case?

Yes, working in it.

Thanks for the patch!

lib/Analysis/MemorySSAUpdater.cpp
126

If we instead refactor this else block to look something like:

auto End = getWritableBlockAccesses()->rend();
for (auto &U : make_range(++MA->getReverseIterator(), End))
  if (!isa<MemoryUse>(U))
    return cast<MemoryAccess>(&U);
// Note that if MA comes before Defs->begin(), we won't hit a def.
return nullptr;

does that still fix the problem?

Personally, I feel like that's clearer about the intent, and is probably less prone to edge-cases.

126

Oops; I meant:

auto End = getWritableBlockAccesses(MA->getBlock())->rend();
dberlin added inline comments.Jun 6 2017, 1:39 PM
lib/Analysis/MemorySSAUpdater.cpp
126

I was going to suggest the same thing george did :)

asbirlea updated this revision to Diff 101625.Jun 6 2017, 2:31 PM

Updated per George's suggestion.
Added unit test.

george.burgess.iv accepted this revision.Jun 6 2017, 8:10 PM

Thanks again!

This revision is now accepted and ready to land.Jun 6 2017, 8:10 PM
This revision was automatically updated to reflect the committed changes.