Fold some expressions early in importer

[EgorBo]

No description provided.

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak
See info in area-owners.md if you want to be subscribed.

Issue Details

---

null

Author:	EgorBo
Assignees:	EgorBo
Labels:	area-CodeGen-coreclr
Milestone:	-

[EgorBo]

@dotnet/jit-contrib single-line change, slightly improves TP and mostly negative diffs.
Size regressions seem to be due to cse, assert prop and branch opts misbehaviors e.g. https://www.diffchecker.com/g6dvRgEN/

TP wins are due to the fact that it now (in some places) removes dead branches earlier than previously and doesn't waste time performing SSA/VN on those, I have a concrete example if you need one.

[tannergooding]

Is it worth having a helper impPopStackAndFoldExpr()?

We can then mark it as forceinline and have it do the two step thing + relevant optimization checks?

[tannergooding]

Alternatively, should we just have foldExpr be a more general part of impPushStack(node)?

We already do explicit gtFoldExpr as part of pushing several node types up, but not more generally.

[EgorBo]

Probably? I assume it needs to be justified in other places as in general it could be a TP regression to run gtFoldExpr where it won't fold anything. But in this place it makes sense to do because BRTRUE/BRFALSE are branches + condition so we potentially remove a branch early

[AndyAyersMS]

This is similar to what we already do for other branches so seems fine.

Doing this more broadly, as Egor says, may not accomplish much other than make the jit slower. But it's probably worth prototyping.

[tannergooding]

Nice diffs!

[AndyAyersMS]

There are small number of minopts diffs, any idea why?

[EgorBo]

There are small number of minopts diffs, any idea why?

Most paths in gtFoldExpr are guarded with opts.OptimizationEnabled() except gtFoldExprConditional - can slap one there as well. Do you mind if I do it separately? We ran an experiment with early out with OptimizationEnabled and it demonstrated nice wins for MinOpts TP but it turns out we first need to make OptimizationEnabled() slightly cheaper than it is now

[AndyAyersMS]

Do you mind if I do it separately?

That's fine. It makes it a little harder to be confident there are no net minopts diffs, but I suppose it's possible we've been doing opts already we didn't intend to do and so the diffs in the follow-up may be more than just undoing the ones here.

[EgorBo]

Do you mind if I do it separately?

That's fine. It makes it a little harder to be confident there are no net minopts diffs, but I suppose it's possible we've been doing opts already we didn't intend to do and so the diffs in the follow-up may be more than just undoing the ones here.

Exactly - we do it in other places as well, a local experiment demonstrated nice wins for early out in gtFoldExrp (up to -0.45%) for MinOpts but first we need to make opts.OptimizationEnabled cheaper