This is an archive of the discontinued LLVM Phabricator instance.

[SCEVExpander] Skip creating <u 0 check, which is always false.
ClosedPublic

Authored by fhahn on Jan 7 2022, 7:23 AM.

Details

Summary

Unsigned compares of the form <u 0 are always false. Do not create such
a redundant check in generateOverflowCheck.

The patch introduces a new lambda to create the check, so we can
exit early conveniently and skip creating some instructions feeding the
check.

I am planning to sink a few additional instructions as follow-ups, but I
would prefer to do this separately, to keep the changes and diff
smaller.

Diff Detail

Event Timeline

fhahn created this revision.Jan 7 2022, 7:23 AM
fhahn requested review of this revision.Jan 7 2022, 7:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 7 2022, 7:23 AM
nikic added inline comments.Jan 7 2022, 8:35 AM
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
2653

FWIW CreateOr already folds LHS | 0 -> LHS, so I think it would make sense to also fold 0 | RHS -> RHS there, for the sake of consistency.

reames accepted this revision.Jan 7 2022, 9:02 AM

LGTM

I agree with Nikita's inline comment, but don't want perfection to the be the enemy of the good here.

This revision is now accepted and ready to land.Jan 7 2022, 9:02 AM
fhahn updated this revision to Diff 398169.Jan 7 2022, 9:15 AM

Remove foldOrCreateOr helper, add logic to IRBuilder in D116817.

fhahn marked an inline comment as done.Jan 7 2022, 9:24 AM
fhahn added inline comments.
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
2653

I moved the fold to D116817. We can discuss that there and if it turns out that it's not desirable to have it in IRBuilder directly I can add it to the expander as follow-up.

fhahn updated this revision to Diff 398171.Jan 7 2022, 9:25 AM
fhahn marked an inline comment as done.

Add back anonymous namespace accidentially dropped in the previous version.