This is an archive of the discontinued LLVM Phabricator instance.

[SCEVExpand] Do not create redundant 'or false' for pred expansion.
ClosedPublic

Authored by fhahn on Jan 5 2022, 1:28 PM.

Details

Summary

This patch updates SCEVExpander::expandWrapPredicate to not create
redundant 'or false, x' instructions. While those are trivially
foldable, they can be easily avoided and hinder code that checks the
size/cost of the generated checks before further folds.

I am planning on look into a few other similar improvements to code
generated by SCEVExpander.

I remember a while ago @lebedev.ri working on doing some trivial folds
like that in IRBuilder itself, but there where concerns that such
changes may subtly break existing code. AFAIK this effort is not active
any longer?

Diff Detail

Event Timeline

fhahn created this revision.Jan 5 2022, 1:28 PM
fhahn requested review of this revision.Jan 5 2022, 1:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 5 2022, 1:28 PM
lebedev.ri accepted this revision.Jan 5 2022, 1:31 PM

SGTM, but i'm not sure the issue of the checks being non-instcombined before their cost being queried can be solved by enhancing SCEVExpander.

This revision is now accepted and ready to land.Jan 5 2022, 1:31 PM
reames accepted this revision.Jan 5 2022, 1:33 PM
reames added inline comments.
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
2590

If you wanted, there's also a CreateOr version which takes an arrayref of operands and does this expansion for you.

This revision was landed with ongoing or failed builds.Jan 6 2022, 3:53 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked an inline comment as done.
fhahn added inline comments.Jan 6 2022, 7:45 AM
llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
2590

Thanks, the committed version uses this version.