Consider refactoring of Compiler::numArgsOfHWIntrinsic function to improve performance

[4creators]

See discussion from this comment on:

dotnet/coreclr#16736 (comment)

category:implementation
theme:intrinsics
skill-level:intermediate
cost:small

[CarolEidt]

Before we spend any time on this, I would like to see some evidence that this actually matters.

[mikedn]

Before we spend any time on this, I would like to see some evidence that this actually matters.

Yep, this whole story is rather speculative. Intrinsic usage will always be pretty low compared to "normal" code so it will be difficult to measure the impact of such a change. It's probably good but too small to matter, especially when comparing it with other zillion of perf issues the JIT has.

Not to mention that coreclr uses PGO and that has a significant impact on inlining and since there's no intrinsic usage PGO may very well decide not to inline even the "improved" function...

[CarolEidt]

In looking at the code, one refactoring that might actually be useful would be to factor out the code that actually counts the node's operands, e.g. countArgsOfHWIntrinsic(GenTreeHWIntrinsic* node). Then, in the case where the numArgs field is not -1, you could add an assert that it returns the same value as numArgs. I suspect it might be useful for asserts elsewhere as well.

[CarolEidt]

Not to mention that coreclr uses PGO and that has a significant impact on inlining and since there's no intrinsic usage PGO may very well decide not to inline even the "improved" function...

Yes, and PGO is a bit tricky because unless you re-run the measurement pass, making changes to the source and rebuilding wont' actually show you what the optimizer will do.

[4creators]

one refactoring that might actually be useful would be to factor out the code that actually counts the node's operands, e.g. countArgsOfHWIntrinsic(GenTreeHWIntrinsic* node).

This is exactly what I wanted to test.

Yes, and PGO is a bit tricky because unless you re-run the measurement pass, making changes to the source and rebuilding wont' actually show you what the optimizer will do.

Yep, that's the problem.

Before we spend any time on this, I would like to see some evidence that this actually matters.

Agree, will run some profiles to check the impact and potential savings

[CarolEidt]

Closing this, as the code has been somewhat refactored, and there doesn't seem to be evidence that this merits further work.