Uh oh!
There was an error while loading. Please reload this page.
Import lzcnt/tzcnt/popcnt as returning int so rngchk works - #128658
Import lzcnt/tzcnt/popcnt as returning int so rngchk works#128658tannergooding wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR changes JIT import/codegen for count-related intrinsics so their IR result can be TYP_INT while still preserving operand width where needed for correct range-check handling.
Changes:
- Imports LZCNT/TZCNT/POPCNT results as
TYP_INTand casts back to the signature return type when needed. - Adds xarch metadata/codegen plumbing to preserve operand width for scalar count instructions.
- Adds ARM64 primitive codegen/LSRA handling for PopCount and TrailingZeroCount.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/coreclr/jit/importercalls.cpp | Updates primitive intrinsic import for LZCNT, TZCNT, POPCNT, and Log2. |
src/coreclr/jit/hwintrinsiclistxarch.h | Marks selected scalar xarch intrinsics as deriving base type from the first argument. |
src/coreclr/jit/hwintrinsiccodegenxarch.cpp | Adjusts xarch count intrinsic codegen to accept an explicit emit size. |
src/coreclr/jit/hwintrinsic.cpp | Adds cast wrapping for selected direct hardware intrinsics returning long-sized values. |
src/coreclr/jit/codegenarmarch.cpp | Adds ARM64 codegen for primitive PopCount and TrailingZeroCount. |
src/coreclr/jit/lsraarm64.cpp | Adds ARM64 LSRA internal SIMD temp allocation for primitive PopCount. |
src/coreclr/jit/codegen.h | Updates the genXCNTIntrinsic declaration. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
EgorBo
commented
May 27, 2026
what kind of rngchk? |
@EgorBo, Range check only works with I'm still waiting to see SPMI diffs and expect we'll also need to get diffs after #128620 goes in (which ensures range check actually handles But initial investigations showed that this is why Arm64 had |
Is there a minimal repro? pretty sure it can be easily fixed in the GetRangeFromAssertions or GetRange vs manual expansions, new QMARKs, etc. E.g. GetRangeFromAssertions doesn't have to bail on TYP_LONG if known range fits into TYP_INT |
tannergooding
commented
May 27, 2026
The only new qmark is from handling This also then ensures that
You can take your pick from the SPMI diffs in the other PR. It's clearly a broadly applicable scenario
Local testing had shown this to be much more expensive and error prone as basically all of range check presumes |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
tannergooding
commented
May 28, 2026
Pulled a couple of the changes out into their own PRs
And then put up a PR that has Will see if I can also get Going to keep this PR in draft and aim to get the others merged first. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
tannergooding
commented
Jun 12, 2026
I'm expecting the other work done allows this to be closed now, but resolved conflicts and want to run SPMI again to confirm |
| // rangecheck requires the type to be TYP_INT to handle this, but the operation may expect TYP_LONG | ||
| // so we fixup the baseType here and in remove the cast in lowering if its unnecessary | ||
| baseType = TYP_INT; |
| // rangecheck requires the type to be TYP_INT to handle this, but the operation may expect TYP_LONG | ||
| // so we fixup the baseType here and in remove the cast in lowering if its unnecessary | ||
| baseType = TYP_INT; |
| // rangecheck requires the type to be TYP_INT to handle this, but the operation may expect TYP_LONG | ||
| // so we fixup the baseType here and in remove the cast in lowering if its unnecessary | ||
| baseType = TYP_INT; |
| if (HWIntrinsicInfo::BaseTypeFromFirstArg(intrinsicId)) | ||
| { | ||
| assert(op1 != nullptr); | ||
| targetType = genActualType(op1->TypeGet()); | ||
| } | ||
| else | ||
| { | ||
| assert(!HWIntrinsicInfo::BaseTypeFromFirstArg(intrinsicId)); | ||
| } |
| // rangecheck requires the type to be TYP_INT to handle this, but the operation may expect | ||
| // TYP_LONG so we fixup the baseType here and in remove the cast in lowering if its | ||
| // unnecessary |
No description provided.