This is an archive of the discontinued LLVM Phabricator instance.

[IndVars] Expand icmp in preheader rather than in loop
ClosedPublic

Authored by aleksandr.popov on Jan 18 2023, 12:06 AM.

Details

Summary

The motivation is that 'createInvariantCond' unconditionally builds icmp in the loop block,
while it could always do it in preheader.
Check that icmp's inputs are loop invariants and build it in preheader.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJan 18 2023, 12:06 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
aleksandr.popov requested review of this revision.Jan 18 2023, 12:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 18 2023, 12:06 AM
nikic added inline comments.Jan 18 2023, 1:02 AM
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
1362–1364

Shouldn't the Rewriter expand in the preheader as well? Can LHSV/RHSV ever *not* be loop invariant in this function?

mkazantsev added inline comments.Jan 18 2023, 3:21 AM
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
1371

I guess you can assert these facts. LIP is supposed to produce loop-invariant SCEVs and expander, I guess, should always be able to expand it in preheader. Isn't it so?

mkazantsev added inline comments.Jan 18 2023, 3:22 AM
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
1362–1364

They should always be invariant. I also think that expander's insertion point can be in preheader.

update AArch64/widen-loop-comp.ll

assert the fact that LHSV/RHSV are loop invariants

nikic added inline comments.Jan 23 2023, 8:58 AM
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
1362–1364

I still think that we should be adjusting this insertion point as well...

Move expander's insertion point to the preheader

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
1362–1364

Got it, thanks!
Moved expander's insertion point to the preheader

This revision is now accepted and ready to land.Jan 24 2023, 2:28 AM
This revision was landed with ongoing or failed builds.Jan 24 2023, 11:41 PM
This revision was automatically updated to reflect the committed changes.