This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
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.
Adding some new functions to System.Math and System.MathF #20788
Adding some new functions to System.Math and System.MathF #20788
Changes from 4 commits
8cb0330
fb14c5e
11a3d1e
e527d7e
0a613b3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was looking at the codegen for this method:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following, which is produced from
BitConverter.SingleToInt32Bits
Could be:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following, which is produced by
if ((xbits ^ ybits) < 0)
:Could be:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
G_M7953_IG03:
andG_M7953_IG04:
are the same, and could be folded together.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following, which is produced from BitConverter.Int32BitsToSingle
Could be:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means we should be generating something closer to the following, if possible:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, there is no transition penalty in VEX encoding, but we insert
vzeroupper
at the prolog to avoid SSE-to-AVX transition penalty from P/Invoke or corssgen-ed code calling JITed code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The architecture optimization manual seems to imply that there is no transition penalty when going from legacy 128-bit instructions to VEX-encoded 128-bit instructions (or vice-versa). Only when going from legacy 128-bit instructions to VEX-encoded 256-bit instructions (and that you avoid the penalty by using
VZEROUPPER
) and when going from VEX-encoded 256-bit instructions to legacy 128-bit instructions (where you cannot avoid the penalty). -- See the last line in the table, forMixed Code Handling
(and some surrounding text in the optimization manual)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the optimization manual is confusing here. Yes, there is no transition penalty when going from legacy 128-bit instructions to VEX-encoded 128-bit instructions on Skylake (and newer) micro-architectures.
But the penalty still can happen on older architectures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. There is a penalty on Broadwell and prior when going from
Preserved Non-INIT Upper State
toDirty Upper State
orClean Upper State
. However, it looks likePenalty D
andPenalty B
are approx the same (whether it is implicit to dirty, or explicit toclean
viavzeroupper
). The difference is whether there is an additional penalty when transitioning back into the legacy instructions (which will impact crossgen code)...