Page MenuHomePhabricator

[RDA] Fix getUniqueReachingDef for self loops
ClosedPublic

Authored by samparker on Sep 16 2020, 2:20 AM.

Details

Summary

We've fixed the case where this could return an instruction after the given instruction, but also means that we can falsely return a 'unique' def when they could be one coming from the backedge of loop.

Diff Detail

Event Timeline

samparker created this revision.Sep 16 2020, 2:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2020, 2:20 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
samparker requested review of this revision.Sep 16 2020, 2:20 AM
samtebbs added inline comments.Sep 16 2020, 3:06 AM
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
452

Small nit but you could use Parent here to avoid calling MI->getParent() again.

samparker updated this revision to Diff 292166.Sep 16 2020, 3:29 AM

Cheers, now comparing against 'Parent'.

SjoerdMeijer added inline comments.Sep 16 2020, 4:05 AM
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
441–442

nit: after some coding style discussion on the list, I believe it is preferred to put brackets there.

450

nit: produces -> producing?

454

If I am not mistaken, looks like we are actually return nullptr here, except for one case. Thus, can we simplify the above to this below?

if (Def->getParent() != Parent && Incoming.size() == 1)
  return Def;
return nullptr;
samparker updated this revision to Diff 292178.Sep 16 2020, 4:15 AM

Thanks, the logic does simplify quite a bit.

SjoerdMeijer accepted this revision.Sep 16 2020, 4:39 AM

Cheers, LGTM

llvm/lib/CodeGen/ReachingDefAnalysis.cpp
442–445

typos: "with" and "that is does"

This revision is now accepted and ready to land.Sep 16 2020, 4:39 AM
This revision was automatically updated to reflect the committed changes.