This is an archive of the discontinued LLVM Phabricator instance.

[IRCE] Support inverted range check's predicate
ClosedPublic

Authored by aleksandr.popov on Apr 13 2023, 9:00 AM.

Details

Summary

IRCE expects true edge of range check's branch comes to loop.
If it meets reverse case - invert the branch.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2023, 9:00 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
aleksandr.popov requested review of this revision.Apr 13 2023, 9:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2023, 9:00 AM
mkazantsev requested changes to this revision.Apr 23 2023, 10:05 PM
mkazantsev added inline comments.
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
401

You can modify IR at this point and then return false here:

for (auto *BBI : L->getBlocks())
  if (BranchInst *TBI = dyn_cast<BranchInst>(BBI->getTerminator()))
    InductiveRangeCheck::extractRangeChecksFromBranch(TBI, L, SE, BPI,
                                                      RangeChecks);
 
if (RangeChecks.empty())
  return false;

it would mean that you modify IR and then report that no modification was done and all analyzes can be preserved, which is wrong.

This revision now requires changes to proceed.Apr 23 2023, 10:05 PM
aleksandr.popov marked an inline comment as done.
skatkov added inline comments.Jun 29 2023, 12:28 AM
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
397

Can we move this transformation AFTER profitability check? The idea is that if we will not add this check to consideration, no need to do modification.

I guess it should work:

  1. Get operand idx of edge coming to loop
  2. use this operand number in getEdgeProbability
  3. if we pass check, do transformation if operand idx != 0.
1925–1926

Is it Dead variable?

Any words in git commit message?

skatkov added inline comments.Jun 29 2023, 12:49 AM
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
397

No operand idx but successor idx...

aleksandr.popov edited the summary of this revision. (Show Details)
aleksandr.popov marked 2 inline comments as done.Jul 3 2023, 2:02 PM
aleksandr.popov added inline comments.
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
397

Good point, thanks!

1925–1926
skatkov accepted this revision.Jul 3 2023, 8:33 PM

In the commit message adds a couple of words about the reason - IRCE expects that true branch comes to loop.

skatkov added inline comments.Jul 3 2023, 8:35 PM
llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
405

dot at the end, please.

aleksandr.popov marked an inline comment as done.
aleksandr.popov edited the summary of this revision. (Show Details)
aleksandr.popov marked 2 inline comments as done.Jul 5 2023, 12:25 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jul 5 2023, 12:49 AM
This revision was automatically updated to reflect the committed changes.