Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit bbe202b

Browse files
author
Sergey Andreenko
authored
small clean-up and bug fix in genIntToIntCast (#10989)
genintToIntCast was cleaned-up, the assert that was wrong for x86 was deleted. The repro was added.
1 parent 9051ff8 commit bbe202b

File tree

3 files changed

+150
-57
lines changed

3 files changed

+150
-57
lines changed

‎src/jit/codegenxarch.cpp‎

Lines changed: 30 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -6344,7 +6344,7 @@ void CodeGen::genSetRegToCond(regNumber dstReg, GenTreePtr tree)
63446344

63456345
#if !defined(_TARGET_64BIT_)
63466346
//------------------------------------------------------------------------
6347-
// genIntToIntCast: Generate code for long to int casts on x86.
6347+
// genLongToIntCast: Generate code for long to int casts on x86.
63486348
//
63496349
// Arguments:
63506350
// cast - The GT_CAST node
@@ -6454,14 +6454,15 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
64546454

64556455
GenTreePtr castOp = treeNode->gtCast.CastOp();
64566456
var_types srcType = genActualType(castOp->TypeGet());
6457+
noway_assert(genTypeSize(srcType) >= 4);
64576458

6458-
#if !defined(_TARGET_64BIT_)
6459+
#ifdef _TARGET_X86_
64596460
if (varTypeIsLong(srcType))
64606461
{
64616462
genLongToIntCast(treeNode);
64626463
return;
64636464
}
6464-
#endif // !defined(_TARGET_64BIT_)
6465+
#endif // _TARGET_X86_
64656466

64666467
regNumber targetReg = treeNode->gtRegNum;
64676468
regNumber sourceReg = castOp->gtRegNum;
@@ -6477,33 +6478,29 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
64776478
}
64786479

64796480
bool requiresOverflowCheck = false;
6480-
bool needAndAfter = false;
64816481

64826482
assert(genIsValidIntReg(targetReg));
64836483
assert(genIsValidIntReg(sourceReg));
64846484

6485-
instruction ins = INS_invalid;
6486-
emitAttr size = EA_UNKNOWN;
6485+
instruction ins = INS_invalid;
6486+
emitAttr srcSize = EA_ATTR(genTypeSize(srcType));
6487+
emitAttr dstSize = EA_ATTR(genTypeSize(dstType));
64876488

6488-
if (genTypeSize(srcType) < genTypeSize(dstType))
6489+
if (srcSize < dstSize)
64896490
{
64906491
// Widening cast
6491-
64926492
// Is this an Overflow checking cast?
64936493
// We only need to handle one case, as the other casts can never overflow.
64946494
// cast from TYP_INT to TYP_ULONG
64956495
//
64966496
if (treeNode->gtOverflow() && (srcType == TYP_INT) && (dstType == TYP_ULONG))
64976497
{
64986498
requiresOverflowCheck = true;
6499-
size = EA_ATTR(genTypeSize(srcType));
65006499
ins = INS_mov;
65016500
}
65026501
else
65036502
{
6504-
// we need the source size
6505-
size = EA_ATTR(genTypeSize(srcType));
6506-
noway_assert(size < EA_PTRSIZE);
6503+
noway_assert(srcSize < EA_PTRSIZE);
65076504

65086505
ins = ins_Move_Extend(srcType, castOp->InReg());
65096506

@@ -6513,44 +6510,30 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
65136510
64-bit, and a regular 32-bit mov clears the high 32 bits (like the non-existant movzxd),
65146511
but for a sign extension from TYP_INT to TYP_LONG, we need to use movsxd opcode.
65156512
*/
6516-
if (!isUnsignedSrc && !isUnsignedDst && (size == EA_4BYTE) && (genTypeSize(dstType) > EA_4BYTE))
6513+
if (!isUnsignedSrc && !isUnsignedDst)
65176514
{
65186515
#ifdef _TARGET_X86_
65196516
NYI_X86("Cast to 64 bit for x86/RyuJIT");
65206517
#else // !_TARGET_X86_
65216518
ins = INS_movsxd;
65226519
#endif // !_TARGET_X86_
65236520
}
6524-
6525-
/*
6526-
Special case: for a cast of byte to char we first
6527-
have to expand the byte (w/ sign extension), then
6528-
mask off the high bits.
6529-
Use 'movsx' followed by 'and'
6530-
*/
6531-
if (!isUnsignedSrc && isUnsignedDst && (genTypeSize(dstType) < EA_4BYTE))
6532-
{
6533-
noway_assert(genTypeSize(dstType) == EA_2BYTE && size == EA_1BYTE);
6534-
needAndAfter = true;
6535-
}
65366521
}
65376522
}
65386523
else
65396524
{
65406525
// Narrowing cast, or sign-changing cast
6541-
noway_assert(genTypeSize(srcType) >= genTypeSize(dstType));
6526+
noway_assert(srcSize >= dstSize);
65426527

65436528
// Is this an Overflow checking cast?
65446529
if (treeNode->gtOverflow())
65456530
{
65466531
requiresOverflowCheck = true;
6547-
size = EA_ATTR(genTypeSize(srcType));
65486532
ins = INS_mov;
65496533
}
65506534
else
65516535
{
6552-
size = EA_ATTR(genTypeSize(dstType));
6553-
ins = ins_Move_Extend(dstType, castOp->InReg());
6536+
ins = ins_Move_Extend(dstType, castOp->InReg());
65546537
}
65556538
}
65566539

@@ -6632,7 +6615,7 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
66326615
if (signCheckOnly)
66336616
{
66346617
// We only need to check for a negative value in sourceReg
6635-
inst_RV_IV(INS_cmp, sourceReg, 0, size);
6618+
inst_RV_IV(INS_cmp, sourceReg, 0, srcSize);
66366619
genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW);
66376620
}
66386621
else
@@ -6645,13 +6628,13 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
66456628
{
66466629
regNumber tmpReg = treeNode->GetSingleTempReg();
66476630
inst_RV_RV(INS_mov, tmpReg, sourceReg, TYP_LONG); // Move the 64-bit value to a writeable temp reg
6648-
inst_RV_SH(INS_SHIFT_RIGHT_LOGICAL, size, tmpReg, 32); // Shift right by 32 bits
6649-
genJumpToThrowHlpBlk(EJ_jne, SCK_OVERFLOW); // Throw if result shift is non-zero
6631+
inst_RV_SH(INS_SHIFT_RIGHT_LOGICAL, srcSize, tmpReg, 32); // Shift right by 32 bits
6632+
genJumpToThrowHlpBlk(EJ_jne, SCK_OVERFLOW); // Throw if result shift is non-zero
66506633
}
66516634
else
66526635
{
66536636
noway_assert(typeMask != 0);
6654-
inst_RV_IV(INS_TEST, sourceReg, typeMask, size);
6637+
inst_RV_IV(INS_TEST, sourceReg, typeMask, srcSize);
66556638
genJumpToThrowHlpBlk(EJ_jne, SCK_OVERFLOW);
66566639
}
66576640
}
@@ -6665,12 +6648,12 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
66656648

66666649
noway_assert((typeMin != 0) && (typeMax != 0));
66676650

6668-
inst_RV_IV(INS_cmp, sourceReg, typeMax, size);
6651+
inst_RV_IV(INS_cmp, sourceReg, typeMax, srcSize);
66696652
genJumpToThrowHlpBlk(EJ_jg, SCK_OVERFLOW);
66706653

66716654
// Compare with the MIN
66726655

6673-
inst_RV_IV(INS_cmp, sourceReg, typeMin, size);
6656+
inst_RV_IV(INS_cmp, sourceReg, typeMin, srcSize);
66746657
genJumpToThrowHlpBlk(EJ_jl, SCK_OVERFLOW);
66756658
}
66766659
}
@@ -6680,15 +6663,13 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
66806663
// On amd64, we can hit this path for a same-register
66816664
// 4-byte to 8-byte widening conversion, and need to
66826665
// emit the instruction to set the high bits correctly.
6683-
|| (EA_ATTR(genTypeSize(dstType)) == EA_8BYTE && EA_ATTR(genTypeSize(srcType)) == EA_4BYTE)
6666+
|| (dstSize == EA_8BYTE && srcSize == EA_4BYTE)
66846667
#endif // _TARGET_AMD64_
66856668
)
6686-
inst_RV_RV(ins, targetReg, sourceReg, srcType, size);
6669+
inst_RV_RV(ins, targetReg, sourceReg, srcType, srcSize);
66876670
}
66886671
else // non-overflow checking cast
66896672
{
6690-
noway_assert(size < EA_PTRSIZE || srcType == dstType);
6691-
66926673
// We may have code transformations that result in casts where srcType is the same as dstType.
66936674
// e.g. Bug 824281, in which a comma is split by the rationalizer, leaving an assignment of a
66946675
// long constant to a long lclVar.
@@ -6697,7 +6678,7 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
66976678
ins = INS_mov;
66986679
}
66996680
/* Is the value sitting in a non-byte-addressable register? */
6700-
else if (castOp->InReg() && (size == EA_1BYTE) && !isByteReg(sourceReg))
6681+
else if (castOp->InReg() && (dstSize == EA_1BYTE) && !isByteReg(sourceReg))
67016682
{
67026683
if (isUnsignedDst)
67036684
{
@@ -6713,21 +6694,21 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
67136694
/* Generate "mov targetReg, castOp->gtReg */
67146695
if (targetReg != sourceReg)
67156696
{
6716-
inst_RV_RV(INS_mov, targetReg, sourceReg, srcType);
6697+
inst_RV_RV(INS_mov, targetReg, sourceReg, srcType, srcSize);
67176698
}
67186699
}
67196700

67206701
if (ins == INS_AND)
67216702
{
6722-
noway_assert((needAndAfter == false) && isUnsignedDst);
6703+
noway_assert(isUnsignedDst);
67236704

67246705
/* Generate "and reg, MASK */
67256706
unsigned fillPattern;
6726-
if (size == EA_1BYTE)
6707+
if (dstSize == EA_1BYTE)
67276708
{
67286709
fillPattern = 0xff;
67296710
}
6730-
else if (size == EA_2BYTE)
6711+
else if (dstSize == EA_2BYTE)
67316712
{
67326713
fillPattern = 0xffff;
67336714
}
@@ -6741,37 +6722,29 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
67416722
#ifdef _TARGET_AMD64_
67426723
else if (ins == INS_movsxd)
67436724
{
6744-
noway_assert(!needAndAfter);
6745-
inst_RV_RV(ins, targetReg, sourceReg, srcType, size);
6725+
inst_RV_RV(ins, targetReg, sourceReg, srcType, srcSize);
67466726
}
67476727
#endif // _TARGET_AMD64_
67486728
else if (ins == INS_mov)
67496729
{
6750-
noway_assert(!needAndAfter);
67516730
if (targetReg != sourceReg
67526731
#ifdef _TARGET_AMD64_
67536732
// On amd64, 'mov' is the opcode used to zero-extend from
67546733
// 4 bytes to 8 bytes.
6755-
|| (EA_ATTR(genTypeSize(dstType)) == EA_8BYTE && EA_ATTR(genTypeSize(srcType)) == EA_4BYTE)
6734+
|| (dstSize == EA_8BYTE && srcSize == EA_4BYTE)
67566735
#endif // _TARGET_AMD64_
67576736
)
67586737
{
6759-
inst_RV_RV(ins, targetReg, sourceReg, srcType, size);
6738+
inst_RV_RV(ins, targetReg, sourceReg, srcType, srcSize);
67606739
}
67616740
}
67626741
else
67636742
{
67646743
noway_assert(ins == INS_movsx || ins == INS_movzx);
6744+
noway_assert(srcSize >= dstSize);
67656745

67666746
/* Generate "mov targetReg, castOp->gtReg */
6767-
inst_RV_RV(ins, targetReg, sourceReg, srcType, size);
6768-
6769-
/* Mask off high bits for cast from byte to char */
6770-
if (needAndAfter)
6771-
{
6772-
noway_assert(genTypeSize(dstType) == 2 && ins == INS_movsx);
6773-
inst_RV_IV(INS_AND, targetReg, 0xFFFF, EA_4BYTE);
6774-
}
6747+
inst_RV_RV(ins, targetReg, sourceReg, srcType, dstSize);
67756748
}
67766749
}
67776750

‎tests/src/JIT/Regression/JitBlue/DevDiv_406160/DevDiv_406160.il‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
6+
// Metadata version: v4.0.30319
7+
.assembly extern System.Runtime
8+
{
9+
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
10+
.ver 4:2:0:0
11+
}
12+
.assembly extern System.Console
13+
{
14+
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
15+
.ver 4:1:0:0
16+
}
17+
.assembly DevDiv_406160
18+
{
19+
}
20+
21+
.class private auto ansi beforefieldinit Bug.Program
22+
extends [System.Runtime]System.Object
23+
{
24+
.method static char ILGEN_METHOD(int16, unsigned int16, native unsigned int)
25+
{
26+
.maxstack 65535
27+
.locals init (bool, int64, native unsigned int)
28+
IL_0000: ldarg.s 0x00
29+
IL_0002: ldc.i4.1
30+
IL_0015: clt
31+
IL_001b: starg.s 0x00
32+
IL_001d: ldloc 0x0001
33+
IL_0067: ldc.i8 0xc3ec93cfd869ae83
34+
IL_0072: ldc.r8 float64(0xb47a62a75e195a1c)
35+
IL_007c: conv.ovf.u8
36+
IL_007d: ldc.i4.1
37+
IL_0081: conv.ovf.i8.un
38+
IL_0088: div.un
39+
IL_0089: add.ovf.un
40+
IL_008c: ldloc 0x0001
41+
IL_009a: ldc.i8 0x97a27f9613c909c1
42+
IL_00a3: dup
43+
IL_00a4: clt
44+
IL_00a6: shr.un
45+
IL_00a7: xor
46+
IL_00a8: ldarg.s 0x00
47+
IL_00aa: conv.ovf.u8.un
48+
IL_00ab: and
49+
IL_00ac: ldloc.s 0x01
50+
IL_00ae: and
51+
IL_00af: conv.ovf.i2.un
52+
IL_00b0: xor
53+
IL_00cd: conv.i4
54+
IL_00ce: ret
55+
}
56+
57+
.method public hidebysig static int32 Main() cil managed
58+
{
59+
.entrypoint
60+
// Code size 22 (0x16)
61+
.maxstack 8
62+
IL_0001: ldc.i4.0
63+
IL_0002: ldc.i4.0
64+
IL_0003: ldc.i4.0
65+
IL_0004: call char Bug.Program::ILGEN_METHOD(int16, unsigned int16, native unsigned int)
66+
IL_0005: pop
67+
IL_0009: ldstr "Pass"
68+
IL_000e: call void [System.Console]System.Console::WriteLine(string)
69+
IL_0013: ldc.i4.s 100
70+
IL_0015: ret
71+
} // end of method Program::Main
72+
73+
.method public hidebysig specialname rtspecialname
74+
instance void .ctor() cil managed
75+
{
76+
// Code size 7 (0x7)
77+
.maxstack 8
78+
IL_0000: ldarg.0
79+
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
80+
IL_0006: ret
81+
} // end of method Program::.ctor
82+
83+
} // end of class Bug.Program

‎tests/src/JIT/Regression/JitBlue/DevDiv_406160/DevDiv_406160.ilproj‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
4+
<PropertyGroup>
5+
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
6+
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
7+
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
8+
<SchemaVersion>2.0</SchemaVersion>
9+
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
10+
<OutputType>Exe</OutputType>
11+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
12+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
13+
</PropertyGroup>
14+
<!-- Default configurations to help VS understand the configurations -->
15+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
16+
</PropertyGroup>
17+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
18+
</PropertyGroup>
19+
<ItemGroup>
20+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
21+
<Visible>False</Visible>
22+
</CodeAnalysisDependentAssemblyPaths>
23+
</ItemGroup>
24+
<PropertyGroup>
25+
<DebugType>None</DebugType>
26+
<Optimize>True</Optimize>
27+
</PropertyGroup>
28+
<ItemGroup>
29+
<Compile Include="DevDiv_406160.il" />
30+
</ItemGroup>
31+
<ItemGroup>
32+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
33+
</ItemGroup>
34+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
35+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
36+
</PropertyGroup>
37+
</Project>

0 commit comments

Comments
 (0)