summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/jit/assertionprop.cpp47
-rw-r--r--src/jit/gentree.h2
-rw-r--r--tests/src/JIT/Regression/JitBlue/GitHub_6318/GitHub_6318.cs49
-rw-r--r--tests/src/JIT/Regression/JitBlue/GitHub_6318/GitHub_6318.csproj42
-rw-r--r--tests/src/JIT/Regression/JitBlue/GitHub_6318/app.config27
5 files changed, 137 insertions, 30 deletions
diff --git a/src/jit/assertionprop.cpp b/src/jit/assertionprop.cpp
index 004e7d9903..750e2e30db 100644
--- a/src/jit/assertionprop.cpp
+++ b/src/jit/assertionprop.cpp
@@ -3227,25 +3227,13 @@ GenTreePtr Compiler::optAssertionProp_Comma(ASSERT_VALARG_TP assertions
const GenTreePtr tree,
const GenTreePtr stmt)
{
- // Process the bounds check as part of the GT_COMMA node since, we need parent pointer to remove nodes.
- if (tree->gtGetOp1()->OperGet() == GT_ARR_BOUNDS_CHECK)
- {
- // Since the GT_COMMA tree gets processed by assertion prop after the child GT_ARR_BOUNDS_CHECK
- // node in execution order, bounds check assertions will be included for the parent GT_COMMA node.
- // Remove the assertion made by the bounds check tree about itself. Assertion only applies to
- // "future" bounds checks.
- AssertionIndex index = (AssertionIndex)tree->gtGetOp1()->GetAssertion();
- if (index != NO_ASSERTION_INDEX && optGetAssertion(index)->IsBoundsCheckNoThrow())
- {
- BitVecOps::RemoveElemD(apTraits, assertions, index - 1);
- GenTreePtr newTree = optAssertionProp_BndsChk(assertions, tree, stmt);
- BitVecOps::AddElemD(apTraits, assertions, index - 1);
- return newTree;
- }
- else
- {
- return optAssertionProp_BndsChk(assertions, tree, stmt);
- }
+ // Remove the bounds check as part of the GT_COMMA node since we need parent pointer to remove nodes.
+ // When processing visits the bounds check, it sets the throw kind to None if the check is redundant.
+ if ((tree->gtGetOp1()->OperGet() == GT_ARR_BOUNDS_CHECK)
+ && ((tree->gtGetOp1()->gtFlags & GTF_ARR_BOUND_INBND) != 0))
+ {
+ optRemoveRangeCheck(tree, stmt, true, GTF_ASG, true /* force remove */);
+ return optAssertionProp_Update(tree, tree, stmt);
}
return nullptr;
}
@@ -3518,7 +3506,7 @@ GenTreePtr Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, const
return nullptr;
}
- assert(tree->gtOper == GT_COMMA && tree->gtGetOp1()->OperGet() == GT_ARR_BOUNDS_CHECK);
+ assert(tree->gtOper == GT_ARR_BOUNDS_CHECK);
BitVecOps::Iter iter(apTraits, assertions);
unsigned index = 0;
@@ -3533,7 +3521,7 @@ GenTreePtr Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, const
continue;
}
- GenTreeBoundsChk* arrBndsChk = tree->gtGetOp1()->AsBoundsChk();
+ GenTreeBoundsChk* arrBndsChk = tree->AsBoundsChk();
// Set 'isRedundant' to true if we can determine that 'arrBndsChk' can be
// classified as a redundant bounds check using 'curAssertion'
@@ -3609,15 +3597,11 @@ GenTreePtr Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, const
gtDispTree(tree, 0, nullptr, true);
}
#endif
- optRemoveRangeCheck(tree, stmt, true, GTF_ASG, true /* force remove */);
- GenTreePtr newTree = optAssertionProp_Update(tree, tree, stmt);
- if (newTree != nullptr)
- {
- BitVecOps::RemoveElemD(apTraits, assertions, index - 1);
- newTree = optAssertionProp(assertions, tree, stmt);
- BitVecOps::AddElemD(apTraits, assertions, index - 1);
- return newTree;
- }
+
+ // Defer actually removing the tree until processing reaches its parent comma, since
+ // optRemoveRangeCheck needs to rewrite the whole comma tree.
+ arrBndsChk->gtFlags |= GTF_ARR_BOUND_INBND;
+ return nullptr;
}
return nullptr;
}
@@ -3703,6 +3687,9 @@ GenTreePtr Compiler::optAssertionProp(ASSERT_VALARG_TP assertions,
case GT_NULLCHECK:
return optAssertionProp_Ind(assertions, tree, stmt);
+ case GT_ARR_BOUNDS_CHECK:
+ return optAssertionProp_BndsChk(assertions, tree, stmt);
+
case GT_COMMA:
return optAssertionProp_Comma(assertions, tree, stmt);
diff --git a/src/jit/gentree.h b/src/jit/gentree.h
index 99ff508bdd..dd02d91464 100644
--- a/src/jit/gentree.h
+++ b/src/jit/gentree.h
@@ -844,6 +844,8 @@ public:
#define GTF_NO_OP_NO 0x80000000 // GT_NO_OP --Have the codegenerator generate a special nop
+ #define GTF_ARR_BOUND_INBND 0x80000000 // GT_ARR_BOUNDS_CHECK -- have proved this check is always in-bounds
+
//----------------------------------------------------------------
#define GTF_STMT_CMPADD 0x80000000 // GT_STMT -- added by compiler
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_6318/GitHub_6318.cs b/tests/src/JIT/Regression/JitBlue/GitHub_6318/GitHub_6318.cs
new file mode 100644
index 0000000000..af6bfe4015
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_6318/GitHub_6318.cs
@@ -0,0 +1,49 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+using System.Runtime.CompilerServices;
+using System.Numerics;
+
+namespace N
+{
+ public static class C
+ {
+ public static int Main(string[] args)
+ {
+ // Regression test for an issue with assertion prop leading
+ // to the wrong exception being thrown from Vector<T>.CopyTo
+ try
+ {
+ Foo(Vector<int>.Zero);
+ }
+ catch (System.ArgumentOutOfRangeException)
+ {
+ // Caught the right exception
+ return 100;
+ }
+ catch
+ {
+ // Caught the wrong exception
+ return -1;
+ }
+ // Caught no exception
+ return -2;
+ }
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ static int Foo(Vector<int> vec)
+ {
+ int[] a = new int[5];
+ // The index [5] is outside the bounds of array 'a',
+ // so this should throw ArgumentOutOfRangeException.
+ // There's a subsequent check for whether the destination
+ // has enough space to receive the vector, which would
+ // raise an ArgumentException; the bug was that assertion
+ // prop was using the later exception check to prove the
+ // prior one "redundant" because the commas confused the
+ // ordering.
+ vec.CopyTo(a, 5);
+ return a[0];
+ }
+ }
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_6318/GitHub_6318.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_6318/GitHub_6318.csproj
new file mode 100644
index 0000000000..ac9ad0e0df
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_6318/GitHub_6318.csproj
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+ <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+ <PropertyGroup>
+ <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+ <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+ <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+ <SchemaVersion>2.0</SchemaVersion>
+ <ProjectGuid>{80B4796D-0D4C-46A3-9185-6EEA11DD4090}</ProjectGuid>
+ <OutputType>Exe</OutputType>
+ <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+ <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+
+ <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+ </PropertyGroup>
+ <!-- Default configurations to help VS understand the configurations -->
+ <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+ </PropertyGroup>
+ <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+ </PropertyGroup>
+ <PropertyGroup>
+ <DebugType></DebugType>
+ <Optimize>True</Optimize>
+ </PropertyGroup>
+ <ItemGroup>
+ <Compile Include="$(MSBuildProjectName).cs" />
+ </ItemGroup>
+ <ItemGroup>
+ <None Include="$(JitPackagesConfigFileDirectory)threading+thread\project.json" />
+ <None Include="app.config" />
+ </ItemGroup>
+ <ItemGroup>
+ <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+ </ItemGroup>
+ <PropertyGroup>
+ <ProjectJson>$(JitPackagesConfigFileDirectory)threading+thread\project.json</ProjectJson>
+ <ProjectLockJson>$(JitPackagesConfigFileDirectory)threading+thread\project.lock.json</ProjectLockJson>
+ </PropertyGroup>
+ <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+ <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+ </PropertyGroup>
+</Project>
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_6318/app.config b/tests/src/JIT/Regression/JitBlue/GitHub_6318/app.config
new file mode 100644
index 0000000000..6f7bbd9d2b
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_6318/app.config
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="utf-8"?>
+<configuration>
+ <runtime>
+ <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
+ <dependentAssembly>
+ <assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+ <bindingRedirect oldVersion="0.0.0.0-4.0.20.0" newVersion="4.0.20.0" />
+ </dependentAssembly>
+ <dependentAssembly>
+ <assemblyIdentity name="System.Text.Encoding" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+ <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+ </dependentAssembly>
+ <dependentAssembly>
+ <assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+ <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+ </dependentAssembly>
+ <dependentAssembly>
+ <assemblyIdentity name="System.IO" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+ <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+ </dependentAssembly>
+ <dependentAssembly>
+ <assemblyIdentity name="System.Reflection" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+ <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+ </dependentAssembly>
+ </assemblyBinding>
+ </runtime>
+</configuration>
generated by cgit v1.2.3 (git 2.25.1) at 2024-09-01 01:46:25 +0000