Improve compare-and-branch sequences produced by Emitter #111797
Conversation
ghost
added
the
area-CodeGen-coreclr
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
kunalspathak
left a comment
kunalspathak
left a comment
There was a problem hiding this comment.
Added few comments.
src/coreclr/jit/codegencommon.cpp
Outdated
| BasicBlock* CodeGen::genGetThrowHelper(SpecialCodeKind codeKind) | ||
| { | ||
| bool useThrowHlpBlk = compiler->fgUseThrowHelperBlocks(); | ||
| #if defined(UNIX_X86_ABI) |
There was a problem hiding this comment.
can we remove this s because it is mainly used for arm64. Perhaps move this code from codegencommon.cpp to codegenarm64.cpp?
There was a problem hiding this comment.
Yes, I'll do that.
| // For ARM64 we only expect equality comparisons. | ||
| assert((cond == GenCondition::EQ) || (cond == GenCondition::NE)); | ||
|
|
||
| if (compareImm == 0) |
There was a problem hiding this comment.
this method is just called with compareImm == 0. Are you planning to use it for other scenarios as well?
There was a problem hiding this comment.
Yes I would try and use this helper anywhere I have an immediate and want to compare-and-branch as it would handle the instruction selection generally.
Maybe if we used it here it might emit tbnz sometimes for 2^n? Otherwise it just simplifies that block.
runtime/src/coreclr/jit/codegenarm64.cpp
Line 4177 in adf123c
Possibly another
cbz case:runtime/src/coreclr/jit/codegenarm64.cpp
Line 4319 in adf123c
There was a problem hiding this comment.
Do not want to rerun CI for this, but in your next patch, can you please add method summary docs for this method?
Introduces an overload of `genJumpToThrowHlpBlk` that allows you to pass a function that generates the branch code, before it creates an inline throw block. This allows us to choose compare-and-branch sequences such as `cbz` on ARM64 for checks added in the emitter, such as divide-by-zero.
Emit `cbz` instead of `cmp+b.ls` when checking bounds for an access to the 0 index of an array. It can only throw when `arrayLength == 0`. Fixes dotnet#42514
snickolls-arm
force-pushed
the
improve-emitter-compare-branch
branch
from
d0db162 to
10e7929
Compare
kunalspathak
left a comment
kunalspathak
left a comment
There was a problem hiding this comment.
Very nice impressive diffs. Thanks for your contribution.
| // For ARM64 we only expect equality comparisons. | ||
| assert((cond == GenCondition::EQ) || (cond == GenCondition::NE)); | ||
|
|
||
| if (compareImm == 0) |
There was a problem hiding this comment.
Do not want to rerun CI for this, but in your next patch, can you please add method summary docs for this method?
* main: JIT: Set PGO data inconsistent when flow disappears in cast expansion (dotnet#112147) [H/3] Fix handling H3_NO_ERROR (dotnet#112125) Change some workflows using `pull_request` to use `pull_request_target` instead (dotnet#112161) Annotate ConfiguredCancelableAsyncEnumerable T with allows ref struct and update extensions (dotnet#111953) Delete copy of performance pipelines in previous location (dotnet#112113) Optimize BigInteger.Divide (dotnet#96895) Use current STJ in HostModel and remove unnecessary audit suppressions (dotnet#109852) JIT: Unify handling of InstParam argument during inlining (dotnet#112119) Remove unneeded DiagnosticSource content (dotnet#112116) Improve compare-and-branch sequences produced by Emitter (dotnet#111797) Jit: Conditional Escape Analysis and Cloning (dotnet#111473) Re-enable HKDF-SHA3 on Azure Linux Remove fstream usage from corehost (dotnet#111859)
3 participants
Introduces an overload of
genJumpToThrowHlpBlkthat allows you to pass a function that generates the branch code, before it creates an inline throw block. This allows us to choose compare-and-branch sequences such ascbzon ARM64 for checks added in the emitter, such as divide-by-zero.Working towards #68028, I'll be looking into something similar for #42514 next using this helper I've added.