As a suggestion: since this helper doesn't create new code, it doesn't seem to be important whether it is FIFO or LIFO. If that is so, I'd suggest using SmallVector and LIFO algorithm instead. It should have better memory footprint. OK if it does as follow-up.

LGTM

Looks good. Sorry for long delay.

Updated the patch in accordance with comments.

@lebedev.ri I am not sure what your logic implied here; maybe instead of removal of assert this check should be a part of if for profitability reasons.

LGTM

LGTM. Hope it doesn't end up exploding. :)

LGTM

LGTM

LGTM

LGTM

Not sure what are cost implications of that, but fine.

LGTM

WOW! Thanks!

Ok, thanks for clarification. Let them shoot their legs then. :) LGTM

Ok, with assert it makes sense. LGTM

I'm not sure about how we should interpret lack of TTI. Maybe we should interpret it like "if there is no model, we don't care about the high costs, and everything is low cost enough", or opposite (like in your patch). Neither of this looks NFC to me (given that we can pass null TTI). Maybe we should merge it *after* we make use of TTI inside the methods?

There is no possible situation when it matters, but fine. Making this check first sounds reasonable. LGTM.

Done.

LGTM.

LGTM (pls make sure that clang-format is fixed).

Hi Alina,

What was the motivation to make it a function pass? Conceptually it's a loop pass.

Do you have a simple test demonstrating a claimed speedup?

I don't understand how come that getOrCreateAddExpr does not do the job instead of your code. Can you please explain what is missing there that is present in your change?

I am a bit worried about that. As far as I remember RightValue needs to be in preheader because it is used in checks that decide whether or not we should go to pre/postloops. Does your case generate them? If yes, please add checks to make it explicit. If not, please construct a similar test with pre/postloop and make sure it passes with the fix.

In the commit message:

I see to possibilities to mitigate that.

to -> two

Just nits from me, I don't know this part well enough to approve.

LGTM with nit (see inline).

Ping?

Ok. Then I suggest to hold it off unless we see a case where unswitch doesn't do its job and this does.

The main motivation is unification - I don't want to leave a single thing that is supported for intrinsic guards and not supported for control flow guards. Unswitching has known problems related to exponential code size growth, so I don't want our ability to eliminate guards in loops to depend on its cost model.

Rebased.

Rebased.