Conversation
|
@dotnet-policy-service agree company="Microsoft" |
d607fee to
285b8e1
Compare
There was a problem hiding this comment.
Pull request overview
Expands ARM64 lowering to produce more conditional-increment (cinc/csinc) patterns, especially for assignments derived from constants compared against a local.
Changes:
- Extend
TryLowerCnsIntCselToCincto also recognizeSELECTCCpatterns where the “+1” value is constant relative to aCMPlocal/constant comparison, and rewrite toGT_SELECT_INCCC. - Broaden
LowerSelect’s trigger to attempt this lowering when either select operand is an integer constant (not only when both are).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/jit/lowerarmarch.cpp | Adds a new lowering path to convert certain SELECTCC + CMP patterns into GT_SELECT_INCCC by substituting a local read for the constVal+1 constant. |
| src/coreclr/jit/lower.cpp | Calls the CINC lowering helper when either select operand is an integer constant, enabling the new pattern matching. |
jakobbotsch
left a comment
There was a problem hiding this comment.
LGTM! I will kick off Fuzzlyn which is good at testing stuff like this (it may have failures in it unrelated to this PR)
|
/azp run Fuzzlyn |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
I went through the Fuzzlyn failures (https://dev.azure.com/dnceng-public/public/_build/results?buildId=1268916&view=ms.vss-build-web.run-extensions-tab) and I don't see anything that obviously looks to be caused by this PR. I think you can bypass the infra issues and merge this. |
c595508 to
1624d87
Compare
|
/ba-g Known arm64 infra issues |
The increment of
lzcnt(*) wasn't getting transformed to a conditional increment, because an earlier phase folds the "add local, 1" into a constant based on assertion information.Fix to allow
TryLowerCnsIntCselToCincto look cases similar toif (myvar==0)myvar=1;fixes #96441