This is an archive of the discontinued LLVM Phabricator instance.

[SCEVExpander] Use IRBuilder to create BinOp
AbandonedPublic

Authored by nikic on Jun 30 2022, 8:01 AM.

Details

Reviewers
fhahn
reames
Summary

This makes InsertBinOp in SCEVExpander go through the IRBuilder, which will fold the operation with InstSimplifyFolder. When doing this, it's important that the flags are directly passed to the builder, not fixed up after the fact, because it might have folded to some unrelated instruction. Expose a CreateNoWrapBinOp API for this purpose.

I am somewhat uncertain whether this change is actually desirable though. This does fold things as expected, but I'm not sure it's wanted in the LSR case, e.g. because it can shift a use from iv.inc to iv. Any thoughts on that?

Diff Detail

Event Timeline

nikic created this revision.Jun 30 2022, 8:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 8:01 AM
nikic requested review of this revision.Jun 30 2022, 8:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 8:01 AM

Unfortunately, I do think the LSR changes are problematic. I suspect this may break assumptions in LSR about register liveness in hard to predict ways. e.g. LoopStrengthReduce/X86/pr46943.ll undoes an optimization to reduce live ranges. (Though, why that particular example wasn't killed by RLEV I have no idea.)

You could make this conditional on LSRMode. You could have two builders, one simplifying and one not. Note sure if that's worth your while or not though.

nikic abandoned this revision.Jul 1 2022, 5:40 AM

Yeah, I think I'm going to drop this for now. What would probably make most sense to me is that the caller of SCEVExpander can provide the builder, so that different users can use different configurations. But that would need some IRBuilder API changes, because we'd still want to use a custom inserter in either case.