This is an archive of the discontinued LLVM Phabricator instance.

[ValueTracking] Fix analyses to update CxtI to be phi's incoming edges' terminators
ClosedPublic

Authored by aqjune on Sep 26 2020, 8:13 AM.

Details

Summary

It was mentioned that D88276 that when a phi node is visited, terminators at their incoming edges should be used for CtxI.
This is a patch that makes two functions (ComputeNumSignBitsImpl, isGuaranteedNotToBeUndefOrPoison) to do so.

Diff Detail

Event Timeline

aqjune created this revision.Sep 26 2020, 8:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2020, 8:13 AM
aqjune requested review of this revision.Sep 26 2020, 8:13 AM
nikic added inline comments.Sep 27 2020, 1:19 AM
llvm/lib/Analysis/ValueTracking.cpp
2978

The separation of the first incoming value here looks unnecessary, write it like this instead?

Tmp = TyBits;
for (unsigned i = 0, e = NumIncomingValues; i != e; ++i) {
  if (Tmp == 1) return Tmp;
  RecQ.CxtI = PN->getIncomingBlock(i)->getTerminator();
  Tmp = std::min(
      Tmp, ComputeNumSignBits(PN->getIncomingValue(i), Depth + 1, RecQ));
}

Not really related to your change, but it does make it more visible.

llvm/unittests/Analysis/ValueTrackingTest.cpp
789

This test looks like it would be passing both before and after your change. You're also not passing TI to the function, so the failure message doesn't really make sense.

aqjune updated this revision to Diff 294588.Sep 27 2020, 6:32 PM

Address comments

aqjune marked 2 inline comments as done.Sep 27 2020, 6:34 PM
aqjune added inline comments.
llvm/unittests/Analysis/ValueTrackingTest.cpp
789

Sorry, my mistake. Fixed

Nit: consider Q.getWithInstruction, no strong feelings.

nikic accepted this revision.Sep 28 2020, 12:47 AM

LGTM

This revision is now accepted and ready to land.Sep 28 2020, 12:47 AM
This revision was landed with ongoing or failed builds.Sep 28 2020, 7:26 AM
This revision was automatically updated to reflect the committed changes.
aqjune marked an inline comment as done.