Branchless codegen for lifted nullable operators#25992
Conversation
| // tempx.HasValue == tempy.HasValue : | ||
| // false; | ||
| // result = (tempx.GetValueOrDefault() == tempy.GetValueOrDefault()) & | ||
| // (tempx.HasValue == tempy.HasValue); |
There was a problem hiding this comment.
❔ Do we do anything special for cases where tempx or tempy is know to be a non-nullable value?
❔ Regardless of the first one, do we have tests showing the behavior where (tempx XOR tempy) is a non-nullable value?
There was a problem hiding this comment.
where tempx or tempy is know to be a non-nullable value?
That still use short circuiting operators,
MakeOptimizedGetValueOrDefault handles the case where either side is not nullable.
where (tempx ^ tempy) is a non-nullable value?
Only comparison operators end up here and the result is bool.
There was a problem hiding this comment.
Only comparison operators end up here and the result is
bool.
Sorry for the confusion. I meant XOR, not the C# operator. As in cases where tempx is nullable or tempy is nullable, but not both.
There was a problem hiding this comment.
Looks like there is not (or I couldn't find it). Will add a test to verify new codegen (in followup prs)
There was a problem hiding this comment.
I do not think this change has any effect on ^, since it is not a comparison operator.
It is worth investigating whether the same optimization is applicable to ^, but I assume that will come as another change.
There was a problem hiding this comment.
Ah, I see. you meant the operands differing in being non-nullable.
Yes, it is worth adding a test.
|
test windows_release_unit64_prtest please |
|
Looks good. Do you have any measurements of the runtime performance change caused by this? |
|
@VSadov Could you please look at this? |
|
If we are going ahead with this, I expect to see similar changes done in VB, unless it already generates code like that. |
|
|
|
The other operators like It may also be worth to follow up with doing this for VB (be careful there - VB does nullable equality differently). |
|
@dotnet-bot test windows_release_unit64_prtest please |
|
Thank you so much @alrz for your contribution! |
Fixes #17261