JIT: extend OptimizeConstCompare to remove TYP_BYTE casts by AndyAyersMS · Pull Request #129361 · dotnet/runtime

AndyAyersMS · 2026-06-13T02:07:33Z

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets (sbyte)x < -64 lower to cmp cl, 0xC0; setl instead of movsx rax, cl; cmp eax, -64; setl. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width.

Fixes#10337.

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixesdotnet#10337.

AndyAyersMS · 2026-06-13T02:07:58Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

This PR extends CoreCLR xarch JIT lowering so Lowering::OptimizeConstCompare can remove TYP_BYTE casts when comparing against constants that fit in INT8, enabling byte-sized compares (avoiding an extra sign-extension) while preserving correctness for the x < 0 / x >= 0 zero-compare codegen special-case. It also adds a JIT regression test covering the relevant signed-byte comparison patterns.

Changes:

Extend Lowering::OptimizeConstCompare (xarch) with a new TYP_BYTE cast-removal path gated on FitsIn<INT8>(const) and excluding const == 0 with LT/GE.
Add a new JIT regression test (Runtime_10337) that validates semantics for various signed-byte constant compare patterns (including memory and truncating sources).
Wire the new test into the merged regression test project (Regression_ro_2.csproj).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lower.cpp	Adds an xarch-only `TYP_BYTE` cast-removal case in `OptimizeConstCompare`, with a correctness bailout for the `LT/GE` compare-to-zero sign-bit optimization.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_10337` test source in the regression project.
src/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs	New regression test validating behavior for signed-byte constant compares (including truncation and memory-load shapes).

tannergooding · 2026-06-13T03:55:43Z

I'm not actually sure this is better here, due to how Intel and AMD tend to treat operations on partial registers. This is particularly true if the small value comes from memory.

I think it would be good to benchmark this and/or consult our friends at Intel to determine what the recommended codegen here is.

tannergooding

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

AndyAyersMS · 2026-06-13T14:33:35Z

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

All the diffs are basically variants of

tannergooding · 2026-06-13T14:56:43Z

All the diffs are basically variants of

Right. The question is really whether or not using a small register as part of a test/compare is ok (I think it is) or if it has a penalty as compared to just doing the sign extension in the first place.

The rules have changed here quite a lot over time and while I think modern processors are fine for this case since it doesn't use the upper bits and isn't further modifying the register, there are many documented cases where using the 8/16-bit form can and will force a delay because the CPU functionally ends up having to insert the "merge with upper 24/16-bits" logic, which it elides if the instruction which writes the small register immediately zero/sign extends the value instead. -- This is notably a feature of APX as well, allowing the extension to be part of the original instruction and fully removing the merge semantics.

AndyAyersMS · 2026-06-14T18:50:29Z

@Ruihan-Yin any concern over the perf impact of this optimization? My simple local tests don't find any unusual stalls.

Ruihan-Yin · 2026-06-16T19:33:48Z

The optimization looks valid from my perspective, CMP/TEST does not update data registers but just EFLAGS registers, so I don't expect they have partial write issue, flag register update should be the same no matter the data size. There should be no uarch level concern for this optimization.

But as Tanner pointed out, 8-bit operation is generally costly for its merging behavior (does not apply to TEST/CMP). And JIT avoids it AFAIK.

AndyAyersMS · 2026-06-17T13:57:21Z

/ba-g persistent timeouts

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixes#10337.

AndyAyersMS · 2026-06-13T02:07:33Z

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets (sbyte)x < -64 lower to cmp cl, 0xC0; setl instead of movsx rax, cl; cmp eax, -64; setl. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width.

Fixes#10337.

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixesdotnet#10337.

AndyAyersMS · 2026-06-13T02:07:58Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

This PR extends CoreCLR xarch JIT lowering so Lowering::OptimizeConstCompare can remove TYP_BYTE casts when comparing against constants that fit in INT8, enabling byte-sized compares (avoiding an extra sign-extension) while preserving correctness for the x < 0 / x >= 0 zero-compare codegen special-case. It also adds a JIT regression test covering the relevant signed-byte comparison patterns.

Changes:

Extend Lowering::OptimizeConstCompare (xarch) with a new TYP_BYTE cast-removal path gated on FitsIn<INT8>(const) and excluding const == 0 with LT/GE.
Add a new JIT regression test (Runtime_10337) that validates semantics for various signed-byte constant compare patterns (including memory and truncating sources).
Wire the new test into the merged regression test project (Regression_ro_2.csproj).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lower.cpp	Adds an xarch-only `TYP_BYTE` cast-removal case in `OptimizeConstCompare`, with a correctness bailout for the `LT/GE` compare-to-zero sign-bit optimization.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_10337` test source in the regression project.
src/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs	New regression test validating behavior for signed-byte constant compares (including truncation and memory-load shapes).

tannergooding · 2026-06-13T03:55:43Z

I'm not actually sure this is better here, due to how Intel and AMD tend to treat operations on partial registers. This is particularly true if the small value comes from memory.

I think it would be good to benchmark this and/or consult our friends at Intel to determine what the recommended codegen here is.

tannergooding

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

AndyAyersMS · 2026-06-13T14:33:35Z

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

All the diffs are basically variants of

tannergooding · 2026-06-13T14:56:43Z

All the diffs are basically variants of

Right. The question is really whether or not using a small register as part of a test/compare is ok (I think it is) or if it has a penalty as compared to just doing the sign extension in the first place.

The rules have changed here quite a lot over time and while I think modern processors are fine for this case since it doesn't use the upper bits and isn't further modifying the register, there are many documented cases where using the 8/16-bit form can and will force a delay because the CPU functionally ends up having to insert the "merge with upper 24/16-bits" logic, which it elides if the instruction which writes the small register immediately zero/sign extends the value instead. -- This is notably a feature of APX as well, allowing the extension to be part of the original instruction and fully removing the merge semantics.

AndyAyersMS · 2026-06-14T18:50:29Z

@Ruihan-Yin any concern over the perf impact of this optimization? My simple local tests don't find any unusual stalls.

Ruihan-Yin · 2026-06-16T19:33:48Z

The optimization looks valid from my perspective, CMP/TEST does not update data registers but just EFLAGS registers, so I don't expect they have partial write issue, flag register update should be the same no matter the data size. There should be no uarch level concern for this optimization.

But as Tanner pointed out, 8-bit operation is generally costly for its merging behavior (does not apply to TEST/CMP). And JIT avoids it AFAIK.

AndyAyersMS · 2026-06-17T13:57:21Z

/ba-g persistent timeouts

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixes#10337.

AndyAyersMS · 2026-06-13T02:07:33Z

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets (sbyte)x < -64 lower to cmp cl, 0xC0; setl instead of movsx rax, cl; cmp eax, -64; setl. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width.

Fixes#10337.

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixesdotnet#10337.

AndyAyersMS · 2026-06-13T02:07:58Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

This PR extends CoreCLR xarch JIT lowering so Lowering::OptimizeConstCompare can remove TYP_BYTE casts when comparing against constants that fit in INT8, enabling byte-sized compares (avoiding an extra sign-extension) while preserving correctness for the x < 0 / x >= 0 zero-compare codegen special-case. It also adds a JIT regression test covering the relevant signed-byte comparison patterns.

Changes:

Extend Lowering::OptimizeConstCompare (xarch) with a new TYP_BYTE cast-removal path gated on FitsIn<INT8>(const) and excluding const == 0 with LT/GE.
Add a new JIT regression test (Runtime_10337) that validates semantics for various signed-byte constant compare patterns (including memory and truncating sources).
Wire the new test into the merged regression test project (Regression_ro_2.csproj).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lower.cpp	Adds an xarch-only `TYP_BYTE` cast-removal case in `OptimizeConstCompare`, with a correctness bailout for the `LT/GE` compare-to-zero sign-bit optimization.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_10337` test source in the regression project.
src/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs	New regression test validating behavior for signed-byte constant compares (including truncation and memory-load shapes).

tannergooding · 2026-06-13T03:55:43Z

I'm not actually sure this is better here, due to how Intel and AMD tend to treat operations on partial registers. This is particularly true if the small value comes from memory.

I think it would be good to benchmark this and/or consult our friends at Intel to determine what the recommended codegen here is.

tannergooding

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

AndyAyersMS · 2026-06-13T14:33:35Z

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

All the diffs are basically variants of

tannergooding · 2026-06-13T14:56:43Z

All the diffs are basically variants of

Right. The question is really whether or not using a small register as part of a test/compare is ok (I think it is) or if it has a penalty as compared to just doing the sign extension in the first place.

The rules have changed here quite a lot over time and while I think modern processors are fine for this case since it doesn't use the upper bits and isn't further modifying the register, there are many documented cases where using the 8/16-bit form can and will force a delay because the CPU functionally ends up having to insert the "merge with upper 24/16-bits" logic, which it elides if the instruction which writes the small register immediately zero/sign extends the value instead. -- This is notably a feature of APX as well, allowing the extension to be part of the original instruction and fully removing the merge semantics.

AndyAyersMS · 2026-06-14T18:50:29Z

@Ruihan-Yin any concern over the perf impact of this optimization? My simple local tests don't find any unusual stalls.

Ruihan-Yin · 2026-06-16T19:33:48Z

The optimization looks valid from my perspective, CMP/TEST does not update data registers but just EFLAGS registers, so I don't expect they have partial write issue, flag register update should be the same no matter the data size. There should be no uarch level concern for this optimization.

But as Tanner pointed out, 8-bit operation is generally costly for its merging behavior (does not apply to TEST/CMP). And JIT avoids it AFAIK.

AndyAyersMS · 2026-06-17T13:57:21Z

/ba-g persistent timeouts

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixes#10337.

AndyAyersMS · 2026-06-13T02:07:33Z

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets (sbyte)x < -64 lower to cmp cl, 0xC0; setl instead of movsx rax, cl; cmp eax, -64; setl. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width.

Fixes#10337.

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixesdotnet#10337.

AndyAyersMS · 2026-06-13T02:07:58Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

This PR extends CoreCLR xarch JIT lowering so Lowering::OptimizeConstCompare can remove TYP_BYTE casts when comparing against constants that fit in INT8, enabling byte-sized compares (avoiding an extra sign-extension) while preserving correctness for the x < 0 / x >= 0 zero-compare codegen special-case. It also adds a JIT regression test covering the relevant signed-byte comparison patterns.

Changes:

Extend Lowering::OptimizeConstCompare (xarch) with a new TYP_BYTE cast-removal path gated on FitsIn<INT8>(const) and excluding const == 0 with LT/GE.
Add a new JIT regression test (Runtime_10337) that validates semantics for various signed-byte constant compare patterns (including memory and truncating sources).
Wire the new test into the merged regression test project (Regression_ro_2.csproj).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lower.cpp	Adds an xarch-only `TYP_BYTE` cast-removal case in `OptimizeConstCompare`, with a correctness bailout for the `LT/GE` compare-to-zero sign-bit optimization.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_10337` test source in the regression project.
src/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs	New regression test validating behavior for signed-byte constant compares (including truncation and memory-load shapes).

tannergooding · 2026-06-13T03:55:43Z

I'm not actually sure this is better here, due to how Intel and AMD tend to treat operations on partial registers. This is particularly true if the small value comes from memory.

I think it would be good to benchmark this and/or consult our friends at Intel to determine what the recommended codegen here is.

tannergooding

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

AndyAyersMS · 2026-06-13T14:33:35Z

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

All the diffs are basically variants of

tannergooding · 2026-06-13T14:56:43Z

All the diffs are basically variants of

Right. The question is really whether or not using a small register as part of a test/compare is ok (I think it is) or if it has a penalty as compared to just doing the sign extension in the first place.

The rules have changed here quite a lot over time and while I think modern processors are fine for this case since it doesn't use the upper bits and isn't further modifying the register, there are many documented cases where using the 8/16-bit form can and will force a delay because the CPU functionally ends up having to insert the "merge with upper 24/16-bits" logic, which it elides if the instruction which writes the small register immediately zero/sign extends the value instead. -- This is notably a feature of APX as well, allowing the extension to be part of the original instruction and fully removing the merge semantics.

AndyAyersMS · 2026-06-14T18:50:29Z

@Ruihan-Yin any concern over the perf impact of this optimization? My simple local tests don't find any unusual stalls.

Ruihan-Yin · 2026-06-16T19:33:48Z

The optimization looks valid from my perspective, CMP/TEST does not update data registers but just EFLAGS registers, so I don't expect they have partial write issue, flag register update should be the same no matter the data size. There should be no uarch level concern for this optimization.

But as Tanner pointed out, 8-bit operation is generally costly for its merging behavior (does not apply to TEST/CMP). And JIT avoids it AFAIK.

AndyAyersMS · 2026-06-17T13:57:21Z

/ba-g persistent timeouts

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixes#10337.

AndyAyersMS · 2026-06-13T02:07:33Z

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets (sbyte)x < -64 lower to cmp cl, 0xC0; setl instead of movsx rax, cl; cmp eax, -64; setl. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width.

Fixes#10337.

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixesdotnet#10337.

AndyAyersMS · 2026-06-13T02:07:58Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

This PR extends CoreCLR xarch JIT lowering so Lowering::OptimizeConstCompare can remove TYP_BYTE casts when comparing against constants that fit in INT8, enabling byte-sized compares (avoiding an extra sign-extension) while preserving correctness for the x < 0 / x >= 0 zero-compare codegen special-case. It also adds a JIT regression test covering the relevant signed-byte comparison patterns.

Changes:

Extend Lowering::OptimizeConstCompare (xarch) with a new TYP_BYTE cast-removal path gated on FitsIn<INT8>(const) and excluding const == 0 with LT/GE.
Add a new JIT regression test (Runtime_10337) that validates semantics for various signed-byte constant compare patterns (including memory and truncating sources).
Wire the new test into the merged regression test project (Regression_ro_2.csproj).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lower.cpp	Adds an xarch-only `TYP_BYTE` cast-removal case in `OptimizeConstCompare`, with a correctness bailout for the `LT/GE` compare-to-zero sign-bit optimization.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_10337` test source in the regression project.
src/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs	New regression test validating behavior for signed-byte constant compares (including truncation and memory-load shapes).

tannergooding · 2026-06-13T03:55:43Z

I'm not actually sure this is better here, due to how Intel and AMD tend to treat operations on partial registers. This is particularly true if the small value comes from memory.

I think it would be good to benchmark this and/or consult our friends at Intel to determine what the recommended codegen here is.

tannergooding

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

AndyAyersMS · 2026-06-13T14:33:35Z

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

All the diffs are basically variants of

tannergooding · 2026-06-13T14:56:43Z

All the diffs are basically variants of

Right. The question is really whether or not using a small register as part of a test/compare is ok (I think it is) or if it has a penalty as compared to just doing the sign extension in the first place.

The rules have changed here quite a lot over time and while I think modern processors are fine for this case since it doesn't use the upper bits and isn't further modifying the register, there are many documented cases where using the 8/16-bit form can and will force a delay because the CPU functionally ends up having to insert the "merge with upper 24/16-bits" logic, which it elides if the instruction which writes the small register immediately zero/sign extends the value instead. -- This is notably a feature of APX as well, allowing the extension to be part of the original instruction and fully removing the merge semantics.

AndyAyersMS · 2026-06-14T18:50:29Z

@Ruihan-Yin any concern over the perf impact of this optimization? My simple local tests don't find any unusual stalls.

Ruihan-Yin · 2026-06-16T19:33:48Z

The optimization looks valid from my perspective, CMP/TEST does not update data registers but just EFLAGS registers, so I don't expect they have partial write issue, flag register update should be the same no matter the data size. There should be no uarch level concern for this optimization.

But as Tanner pointed out, 8-bit operation is generally costly for its merging behavior (does not apply to TEST/CMP). And JIT avoids it AFAIK.

AndyAyersMS · 2026-06-17T13:57:21Z

/ba-g persistent timeouts

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixes#10337.

AndyAyersMS · 2026-06-13T02:07:33Z

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets (sbyte)x < -64 lower to cmp cl, 0xC0; setl instead of movsx rax, cl; cmp eax, -64; setl. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width.

Fixes#10337.

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixesdotnet#10337.

AndyAyersMS · 2026-06-13T02:07:58Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

This PR extends CoreCLR xarch JIT lowering so Lowering::OptimizeConstCompare can remove TYP_BYTE casts when comparing against constants that fit in INT8, enabling byte-sized compares (avoiding an extra sign-extension) while preserving correctness for the x < 0 / x >= 0 zero-compare codegen special-case. It also adds a JIT regression test covering the relevant signed-byte comparison patterns.

Changes:

Extend Lowering::OptimizeConstCompare (xarch) with a new TYP_BYTE cast-removal path gated on FitsIn<INT8>(const) and excluding const == 0 with LT/GE.
Add a new JIT regression test (Runtime_10337) that validates semantics for various signed-byte constant compare patterns (including memory and truncating sources).
Wire the new test into the merged regression test project (Regression_ro_2.csproj).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lower.cpp	Adds an xarch-only `TYP_BYTE` cast-removal case in `OptimizeConstCompare`, with a correctness bailout for the `LT/GE` compare-to-zero sign-bit optimization.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_10337` test source in the regression project.
src/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs	New regression test validating behavior for signed-byte constant compares (including truncation and memory-load shapes).

tannergooding · 2026-06-13T03:55:43Z

I'm not actually sure this is better here, due to how Intel and AMD tend to treat operations on partial registers. This is particularly true if the small value comes from memory.

I think it would be good to benchmark this and/or consult our friends at Intel to determine what the recommended codegen here is.

tannergooding

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

AndyAyersMS · 2026-06-13T14:33:35Z

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

All the diffs are basically variants of

tannergooding · 2026-06-13T14:56:43Z

All the diffs are basically variants of

Right. The question is really whether or not using a small register as part of a test/compare is ok (I think it is) or if it has a penalty as compared to just doing the sign extension in the first place.

The rules have changed here quite a lot over time and while I think modern processors are fine for this case since it doesn't use the upper bits and isn't further modifying the register, there are many documented cases where using the 8/16-bit form can and will force a delay because the CPU functionally ends up having to insert the "merge with upper 24/16-bits" logic, which it elides if the instruction which writes the small register immediately zero/sign extends the value instead. -- This is notably a feature of APX as well, allowing the extension to be part of the original instruction and fully removing the merge semantics.

AndyAyersMS · 2026-06-14T18:50:29Z

@Ruihan-Yin any concern over the perf impact of this optimization? My simple local tests don't find any unusual stalls.

Ruihan-Yin · 2026-06-16T19:33:48Z

The optimization looks valid from my perspective, CMP/TEST does not update data registers but just EFLAGS registers, so I don't expect they have partial write issue, flag register update should be the same no matter the data size. There should be no uarch level concern for this optimization.

But as Tanner pointed out, 8-bit operation is generally costly for its merging behavior (does not apply to TEST/CMP). And JIT avoids it AFAIK.

AndyAyersMS · 2026-06-17T13:57:21Z

/ba-g persistent timeouts

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixes#10337.

AndyAyersMS · 2026-06-13T02:07:33Z

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets (sbyte)x < -64 lower to cmp cl, 0xC0; setl instead of movsx rax, cl; cmp eax, -64; setl. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width.

Fixes#10337.

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixesdotnet#10337.

AndyAyersMS · 2026-06-13T02:07:58Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

This PR extends CoreCLR xarch JIT lowering so Lowering::OptimizeConstCompare can remove TYP_BYTE casts when comparing against constants that fit in INT8, enabling byte-sized compares (avoiding an extra sign-extension) while preserving correctness for the x < 0 / x >= 0 zero-compare codegen special-case. It also adds a JIT regression test covering the relevant signed-byte comparison patterns.

Changes:

Extend Lowering::OptimizeConstCompare (xarch) with a new TYP_BYTE cast-removal path gated on FitsIn<INT8>(const) and excluding const == 0 with LT/GE.
Add a new JIT regression test (Runtime_10337) that validates semantics for various signed-byte constant compare patterns (including memory and truncating sources).
Wire the new test into the merged regression test project (Regression_ro_2.csproj).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lower.cpp	Adds an xarch-only `TYP_BYTE` cast-removal case in `OptimizeConstCompare`, with a correctness bailout for the `LT/GE` compare-to-zero sign-bit optimization.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_10337` test source in the regression project.
src/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs	New regression test validating behavior for signed-byte constant compares (including truncation and memory-load shapes).

tannergooding · 2026-06-13T03:55:43Z

I'm not actually sure this is better here, due to how Intel and AMD tend to treat operations on partial registers. This is particularly true if the small value comes from memory.

I think it would be good to benchmark this and/or consult our friends at Intel to determine what the recommended codegen here is.

tannergooding

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

AndyAyersMS · 2026-06-13T14:33:35Z

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

All the diffs are basically variants of

tannergooding · 2026-06-13T14:56:43Z

All the diffs are basically variants of

Right. The question is really whether or not using a small register as part of a test/compare is ok (I think it is) or if it has a penalty as compared to just doing the sign extension in the first place.

The rules have changed here quite a lot over time and while I think modern processors are fine for this case since it doesn't use the upper bits and isn't further modifying the register, there are many documented cases where using the 8/16-bit form can and will force a delay because the CPU functionally ends up having to insert the "merge with upper 24/16-bits" logic, which it elides if the instruction which writes the small register immediately zero/sign extends the value instead. -- This is notably a feature of APX as well, allowing the extension to be part of the original instruction and fully removing the merge semantics.

AndyAyersMS · 2026-06-14T18:50:29Z

@Ruihan-Yin any concern over the perf impact of this optimization? My simple local tests don't find any unusual stalls.

Ruihan-Yin · 2026-06-16T19:33:48Z

The optimization looks valid from my perspective, CMP/TEST does not update data registers but just EFLAGS registers, so I don't expect they have partial write issue, flag register update should be the same no matter the data size. There should be no uarch level concern for this optimization.

But as Tanner pointed out, 8-bit operation is generally costly for its merging behavior (does not apply to TEST/CMP). And JIT avoids it AFAIK.

AndyAyersMS · 2026-06-17T13:57:21Z

/ba-g persistent timeouts

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixes#10337.

AndyAyersMS · 2026-06-13T02:07:33Z

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets (sbyte)x < -64 lower to cmp cl, 0xC0; setl instead of movsx rax, cl; cmp eax, -64; setl. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width.

Fixes#10337.

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixesdotnet#10337.

AndyAyersMS · 2026-06-13T02:07:58Z

@dotnet/jit-contrib PTAL

Copilot

Pull request overview

This PR extends CoreCLR xarch JIT lowering so Lowering::OptimizeConstCompare can remove TYP_BYTE casts when comparing against constants that fit in INT8, enabling byte-sized compares (avoiding an extra sign-extension) while preserving correctness for the x < 0 / x >= 0 zero-compare codegen special-case. It also adds a JIT regression test covering the relevant signed-byte comparison patterns.

Changes:

Extend Lowering::OptimizeConstCompare (xarch) with a new TYP_BYTE cast-removal path gated on FitsIn<INT8>(const) and excluding const == 0 with LT/GE.
Add a new JIT regression test (Runtime_10337) that validates semantics for various signed-byte constant compare patterns (including memory and truncating sources).
Wire the new test into the merged regression test project (Regression_ro_2.csproj).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lower.cpp	Adds an xarch-only `TYP_BYTE` cast-removal case in `OptimizeConstCompare`, with a correctness bailout for the `LT/GE` compare-to-zero sign-bit optimization.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new `Runtime_10337` test source in the regression project.
src/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs	New regression test validating behavior for signed-byte constant compares (including truncation and memory-load shapes).

tannergooding · 2026-06-13T03:55:43Z

I'm not actually sure this is better here, due to how Intel and AMD tend to treat operations on partial registers. This is particularly true if the small value comes from memory.

I think it would be good to benchmark this and/or consult our friends at Intel to determine what the recommended codegen here is.

tannergooding

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

AndyAyersMS · 2026-06-13T14:33:35Z

The changes look correct to me, but it's probably goodness to dig a little deeper into whether or not this is actually an optimization or not.

All the diffs are basically variants of

tannergooding · 2026-06-13T14:56:43Z

All the diffs are basically variants of

Right. The question is really whether or not using a small register as part of a test/compare is ok (I think it is) or if it has a penalty as compared to just doing the sign extension in the first place.

The rules have changed here quite a lot over time and while I think modern processors are fine for this case since it doesn't use the upper bits and isn't further modifying the register, there are many documented cases where using the 8/16-bit form can and will force a delay because the CPU functionally ends up having to insert the "merge with upper 24/16-bits" logic, which it elides if the instruction which writes the small register immediately zero/sign extends the value instead. -- This is notably a feature of APX as well, allowing the extension to be part of the original instruction and fully removing the merge semantics.

AndyAyersMS · 2026-06-14T18:50:29Z

@Ruihan-Yin any concern over the perf impact of this optimization? My simple local tests don't find any unusual stalls.

Ruihan-Yin · 2026-06-16T19:33:48Z

The optimization looks valid from my perspective, CMP/TEST does not update data registers but just EFLAGS registers, so I don't expect they have partial write issue, flag register update should be the same no matter the data size. There should be no uarch level concern for this optimization.

But as Tanner pointed out, 8-bit operation is generally costly for its merging behavior (does not apply to TEST/CMP). And JIT avoids it AFAIK.

AndyAyersMS · 2026-06-17T13:57:21Z

/ba-g persistent timeouts

For xarch, mirror the existing TYP_UBYTE branch when the const fits in INT8. Lets `(sbyte)x < -64` lower to `cmp cl, 0xC0; setl` instead of `movsx rax, cl; cmp eax, -64; setl`. Skip the transform for op2 == 0 with GT_LT/GT_GE: codegen's sign-bit-shift trick assumes the operand is sign-extended to full register width. Fixes#10337.

CopilotAI review requested due to automatic review settings June 13, 2026 02:07

github-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 13, 2026

dotnet-policy-serviceBot assigned AndyAyersMS Jun 13, 2026

Copilotstarted reviewing on behalf of AndyAyersMS June 13, 2026 02:08 View session

CopilotAI reviewed Jun 13, 2026
View reviewed changes

Comment threadsrc/tests/JIT/Regression/JitBlue/Runtime_10337/Runtime_10337.cs

tannergooding approved these changes Jun 13, 2026
View reviewed changes

AndyAyersMS merged commit 3e4b29a into dotnet:mainJun 17, 2026
143 of 147 checks passed

dotnet-maestroBot mentioned this pull request Jun 18, 2026
[main] Source code updates from dotnet/runtime dotnet/dotnet#7280
Merged

dotnet-milestone-botBot added this to the 11.0-preview6 milestone Jun 18, 2026

github-actionsBot locked and limited conversation to collaborators Jul 19, 2026