This is an archive of the discontinued LLVM Phabricator instance.

[ValueTracking] Handle and/or on RHS of isImpliedCondition()
ClosedPublic

Authored by nikic on May 13 2022, 8:27 AM.

Details

Summary

isImpliedCondition() currently handles and/or on the LHS, but not on the RHS, resulting in asymmetric behavior. This patch adds two new implication rules:

  • LHS ==> (RHS1 || RHS2) if LHS ==> RHS1 or LHS ==> RHS2
  • LHS ==> !(RHS1 && RHS2) if LHS ==> !RHS1 or LHS ==> !RHS2

Diff Detail

Event Timeline

nikic created this revision.May 13 2022, 8:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2022, 8:27 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
nikic requested review of this revision.May 13 2022, 8:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2022, 8:27 AM
spatel accepted this revision.May 16 2022, 5:38 AM

LGTM

llvm/lib/Analysis/ValueTracking.cpp
6675–6679

I had to step through this to convince myself that the logic was correct.

That is probably the shortest way to write it, but something like this might be easier to read?

if (auto ImpR1 = isImpliedCondition(LHS, RHS1, DL, LHSIsTrue, Depth + 1))
  if (*ImpR1 == false)
    return false;
if (auto ImpR2 = isImpliedCondition(LHS, RHS2, DL, LHSIsTrue, Depth + 1))
  if (*ImpR2 == false)
    return false;
This revision is now accepted and ready to land.May 16 2022, 5:38 AM
nikic added inline comments.May 16 2022, 7:19 AM
llvm/lib/Analysis/ValueTracking.cpp
6675–6679

Yeah, I agree that the and case is a bit hard to understand as written.

This revision was landed with ongoing or failed builds.May 16 2022, 7:30 AM
This revision was automatically updated to reflect the committed changes.