Skip to content

Commit

Permalink
Avoid discarding upper bits when folding GT_SWITCH (#69986)
Browse files Browse the repository at this point in the history
  • Loading branch information
jakobbotsch authored Jun 10, 2022
1 parent b22fcf4 commit 933dbb1
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 21 deletions.
7 changes: 6 additions & 1 deletion src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3221,8 +3221,10 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block)
if (verbose)
{
printf("\nConverting a switch (" FMT_BB ") with only one significant clause besides a default target to a "
"conditional branch\n",
"conditional branch. Before:\n",
block->bbNum);

gtDispTree(switchTree);
}
#endif // DEBUG

Expand All @@ -3247,6 +3249,9 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block)
block->bbJumpDest = block->bbJumpSwt->bbsDstTab[0];
block->bbJumpKind = BBJ_COND;

JITDUMP("After:\n");
DISPNODE(switchTree);

return true;
}
return returnvalue;
Expand Down
34 changes: 14 additions & 20 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14922,18 +14922,16 @@ Compiler::FoldResult Compiler::fgFoldConditional(BasicBlock* block)

noway_assert(lastStmt->GetRootNode()->gtOper == GT_SWITCH);

/* Did we fold the conditional */
// Did we fold the conditional

noway_assert(lastStmt->GetRootNode()->AsOp()->gtOp1);
GenTree* condTree;
condTree = lastStmt->GetRootNode()->AsOp()->gtOp1;
GenTree* cond;
cond = condTree->gtEffectiveVal(true);
GenTree* condTree = lastStmt->GetRootNode()->AsOp()->gtOp1;
GenTree* cond = condTree->gtEffectiveVal(true);

if (cond->OperIsConst())
{
/* Yupee - we folded the conditional!
* Remove the conditional statement */
// Yupee - we folded the conditional!
// Remove the conditional statement

noway_assert(cond->gtOper == GT_CNS_INT);

Expand All @@ -14951,17 +14949,13 @@ Compiler::FoldResult Compiler::fgFoldConditional(BasicBlock* block)
result = FoldResult::FOLD_REMOVED_LAST_STMT;
}

/* modify the flow graph */
// modify the flow graph

/* Find the actual jump target */
unsigned switchVal;
switchVal = (unsigned)cond->AsIntCon()->gtIconVal;
unsigned jumpCnt;
jumpCnt = block->bbJumpSwt->bbsCount;
BasicBlock** jumpTab;
jumpTab = block->bbJumpSwt->bbsDstTab;
bool foundVal;
foundVal = false;
// Find the actual jump target
size_t switchVal = (size_t)cond->AsIntCon()->gtIconVal;
unsigned jumpCnt = block->bbJumpSwt->bbsCount;
BasicBlock** jumpTab = block->bbJumpSwt->bbsDstTab;
bool foundVal = false;

for (unsigned val = 0; val < jumpCnt; val++, jumpTab++)
{
Expand All @@ -14976,20 +14970,20 @@ Compiler::FoldResult Compiler::fgFoldConditional(BasicBlock* block)
{
if (curJump != block->bbNext)
{
/* transform the basic block into a BBJ_ALWAYS */
// transform the basic block into a BBJ_ALWAYS
block->bbJumpKind = BBJ_ALWAYS;
block->bbJumpDest = curJump;
}
else
{
/* transform the basic block into a BBJ_NONE */
// transform the basic block into a BBJ_NONE
block->bbJumpKind = BBJ_NONE;
}
foundVal = true;
}
else
{
/* Remove 'block' from the predecessor list of 'curJump' */
// Remove 'block' from the predecessor list of 'curJump'
fgRemoveRefPred(curJump, block);
}
}
Expand Down
107 changes: 107 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_68568/Runtime_68568.il
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

// Metadata version: v4.0.30319
.assembly extern System.Runtime
{
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
.ver 7:0:0:0
}
.assembly extern System.Console
{
.publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....:
.ver 7:0:0:0
}
.assembly playground
{
}

// =============== CLASS MEMBERS DECLARATION ===================

.class public auto ansi beforefieldinit Runtime_68568
extends [System.Runtime]System.Object
{
.method public hidebysig static int32 Main() cil managed
{
.entrypoint
// Code size 79 (0x4f)
.maxstack 2
.locals init (bool V_0)

// Code equivalent to the following, except without the
// int cast that C# adds on switch cases.
//
// switch (unchecked((nuint)0x100000000))
// {
// case 0:
// pass = sizeof(nint) == 4;
// break;
// case 1:
// pass = false;
// break;
// default:
// pass = sizeof(nint) == 8;
// break;
// }
//
// if (pass)
// {
// Console.WriteLine("PASS");
// return 100;
// }
//
// Console.WriteLine("FAIL");
// return -1;


IL_0000: ldc.i4.0
IL_0001: stloc.0
IL_0002: ldc.i8 0x100000000
IL_000b: conv.u
IL_000c: switch (
IL_0025,
IL_0031)
IL_0019: sizeof [System.Runtime]System.IntPtr
IL_001f: ldc.i4.8
IL_0020: ceq
IL_0022: stloc.0
IL_0023: br.s IL_0033

IL_0025: sizeof [System.Runtime]System.IntPtr
IL_002b: ldc.i4.4
IL_002c: ceq
IL_002e: stloc.0
IL_002f: br.s IL_0033

IL_0031: ldc.i4.0
IL_0032: stloc.0
IL_0033: ldloc.0
IL_0034: brfalse.s IL_0043

IL_0036: ldstr "PASS"
IL_003b: call void [System.Console]System.Console::WriteLine(string)
IL_0040: ldc.i4.s 100
IL_0042: ret

IL_0043: ldstr "FAIL"
IL_0048: call void [System.Console]System.Console::WriteLine(string)
IL_004d: ldc.i4.m1
IL_004e: ret
} // end of method Runtime_68568::Main

.method public hidebysig specialname rtspecialname
instance void .ctor() cil managed
{
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [System.Runtime]System.Object::.ctor()
IL_0006: ret
} // end of method Runtime_68568::.ctor

} // end of class Runtime_68568


// =============================================================

// *********** DISASSEMBLY COMPLETE ***********************
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk.IL">
<PropertyGroup>
<OutputType>Exe</OutputType>
</PropertyGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).il" />
</ItemGroup>
</Project>
3 changes: 3 additions & 0 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -2201,6 +2201,9 @@
<ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_70259/Runtime_70259/*">
<Issue>https://github.com/dotnet/runtime/issues/70279</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_68568/Runtime_68568/*">
<Issue>Tests coreclr's handling of switches on natively sized integers</Issue>
</ExcludeList>
</ItemGroup>

<!-- Known failures for mono runtime on Windows -->
Expand Down

0 comments on commit 933dbb1

Please sign in to comment.