Add microbenchmarks for if statements inside loops #3078
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change-Id: I78f97b7e311820288c940e3cc786357cbb8fbe81
|
Results from a 80core Altra. base_src is a recent commit: 2bf8f1aa83e. runtime_src is the same code but with if conversion always enabled inside loops: this block commented out. (yes, this code is already gated on stress, but not sure if the harness allows to run the second coreclr with a different env to the first) Doing if conversion can be expensive due to the values within the if always being calculated. I've tried to factor this out of the benchmarks: the expensive load from the array is always being used via the call to There are some slowdowns where the result of the if is predicable ( Having two conditions in the if ( |
|
@kunalspathak, @jakobbotsch : I'm not sure if this is enough evidence to turn on if conversion inside loops (specifically Arm64). |
|
I would be more worried about the cases where there are data dependencies crossing loop iterations, like in dotnet/runtime#82106 (comment). I think to enable this we would need to do some smart analysis to ensure that the dependency chain the if-conversion appears in is short enough to not cause stalls. |
|
FYI @AndyAyersMS |
Based off that, I gave the following a try. Loop size is unknown. Op1 is used inside the loop, as part of the if and after the loop. I'll have a play, see if I can make it run slower. |
|
Getting back to this. I did some extra testing, but couldn't construct any loops that ran slower. Instead, I did a spmidiff on the patch that enabled if conversion in loops. In the benchmarks mch, I looked at all the functions with the biggest changes. I then tried to find the benchmarks that ran those functions. Firstly, here's the spmi results (attached because the file is too big) |
|
Running benchmarks that test some of the loops functions that get better according to spmidiff: |
|
Running benchmarks that test some of the loops functions that get worse according to spmidiff: |
|
So, some things get faster, so thing get worse. It's not clear that one is obviously better than the other. I would be tempted to leave the code as it is. |
|
Thanks @a74nh for checking.
You mean do not enable conditionals in loop? |
Yes, unless anyone has strong opinions otherwise. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These benchmarks are based heavily on IfLoops.cs.
The intention here is to benchmark the performance of if statements within loops. Specifically, these are being used to test the effect of doing if conversion within loops.
The
*Inner()functions are marked are aggressively inlined - they need to be within the callers code.