Spanify parts of IL reading in ILCompiler #97794
Conversation
* Make ILReader ref struct, only holding IL body bytes and a offset * Use BinaryPrimitives instead of manual bit-shifting * Remove unused ILStreamReader.cs * Make ReadMIbcGroup return List instead of using enumerator
* Remove ReaderExtensions.cs from ILCompiler.Ryujit
ghost
added
area-crossgen2-coreclr
community-contribution
* Use ILReader.ReadBranchDestination to merge two switch branches
MichalStrehovsky
left a comment
MichalStrehovsky
left a comment
There was a problem hiding this comment.
Thank you, looks great!
Cc @jakobbotsch for IBC changes.
|
|
||
| if (!isVariableSize) | ||
| currentOffset += opcode.GetSize(); | ||
| if (!hasReadOperand) |
There was a problem hiding this comment.
Nit: wasn't isVariableSize better? We defaulted to false and had to set to true in one case. Now we still default to false, but need to set it to true in two cases.
There was a problem hiding this comment.
I thought about name of this for a while. I think the meaning of the variable changed because it's using ILReader now.
Before we manually kept track of the currentOffset where isVariableSize was used to skip the ILOpcode.switch_ case in which the currentOffset was calculated manually.
Now by using ILReader.Skip(opcode) (which essentially is operandSize - op.GetSize() + switch handling) we can avoid this duplicated logic, however because we want to read the operand in two cases for stack height tracking purposes (branches and calls/newobj) we need to flip the bit to avoid "reading" the operand twice in ILReader. That was why I called it hasReadOperand.
tldr: the current branches that use the bool are not variable sized, switch was the only special case and we now handle it in ILReader.Skip(op)
This reverts commit f40fcac.
3 participants
Came across some low-hanging fruits while browsing through ILCompiler and decided to try utilize some newer .NET APIs to make the code a bit more "modern". Shouldn't be much of a performance optimization though.
Commits
ILReaderILReaderref struct, only holding IL body bytes and a offsetBinaryPrimitivesinstead of manual bit-shiftingILStreamReader.csReadMIbcGroupreturnList<MethodProfileData>instead of using enumerator, which prevented the use ofILReader(now a ref struct)ILStackHelperILReaderinstead to deduplicate reading logicILReader.ReadBranchDestinationRead7BitEncodedIntReaderExtensions.csfromILCompiler.Ryujit. It wasn't used in the project.Had trouble running NativeAOT smoketests locally because my hardware doesn't support AVX512, which caused
nativeaot.SmokeTests\HardwareIntrinsics\X64Avx512to fail.