When a method returns a multi-reg struct, we set GTF_DONT_CSE on return local: https://github.com/dotnet/coreclr/blob/dotnet/coreclr@
497419bf8f19c649d821295da7e225e55581cce9/src/jit/importer.cpp#L8783
Setting GTF_DONT_CSE blocks assertion propagation here:
https://github.com/dotnet/coreclr/blob/dotnet/coreclr@
9d49bf1ec6f102b89e5c2885e8f9d3d77f2ec144/src/jit/assertionprop.cpp#L2845-L2848
In the test we have a synchronized method so we change the return node to
return a comma that include a call to HELPER.CORINFO_HELP_MON_EXIT.
If the rightmost comma expression doesn't have GTF_DONT_CSE,
assertion propagation is not blocked and we end up with this tree:
```
[000040] -----+------ /--* CNS_INT struct 0
[000043] --C-G+------ /--* COMMA struct
[000036] --C-G+------ | \--* CALL help void HELPER.CORINFO_HELP_MON_EXIT
[000032] L----+------ arg1 in x1 | +--* ADDR long
[000031] ----G+-N---- | | \--* LCL_VAR ubyte (AX) V03 tmp1
[000033] -----+------ arg0 in x0 | \--* LCL_VAR ref V00 this
[000041] -AC-G+------ * COMMA struct
[000006] -----+-N---- | /--* LCL_VAR struct V01 loc0
[000039] -A---+------ \--* ASG struct (copy)
[000037] D----+-N---- \--* LCL_VAR struct V05 tmp3
```
Downstream phases can't handle struct zero return expressed as
```
[000040] -----+------ /--* CNS_INT struct 0
```
The fix is to propagate GTF_DONT_CSE to the rightmost comma expression
to block bad assertion propagation.
Fixes dotnet/coreclr#21011.
Commit migrated from https://github.com/dotnet/coreclr/commit/
e80e04020c3281ec675817c4fff025a3e347353e
--- /dev/null
+diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
+index ad1fd83fe9..bcc818f25b 100644
+--- a/src/jit/flowgraph.cpp
++++ b/src/jit/flowgraph.cpp
+@@ -8016,45 +8016,46 @@ GenTree* Compiler::fgCreateMonitorTree(unsigned lvaMonAcquired, unsigned lvaThis
+ {
+ GenTree* retNode = block->lastStmt()->gtStmtExpr;
+ GenTree* retExpr = retNode->gtOp.gtOp1;
+
+ if (retExpr != nullptr)
+ {
+ // have to insert this immediately before the GT_RETURN so we transform:
+ // ret(...) ->
+ // ret(comma(comma(tmp=...,call mon_exit), tmp)
+ //
+ //
+ // Before morph stage, it is possible to have a case of GT_RETURN(TYP_LONG, op1) where op1's type is
+ // TYP_STRUCT (of 8-bytes) and op1 is call node. See the big comment block in impReturnInstruction()
+ // for details for the case where info.compRetType is not the same as info.compRetNativeType. For
+ // this reason pass compMethodInfo->args.retTypeClass which is guaranteed to be a valid class handle
+ // if the return type is a value class. Note that fgInsertCommFormTemp() in turn uses this class handle
+ // if the type of op1 is TYP_STRUCT to perform lvaSetStruct() on the new temp that is created, which
+ // in turn passes it to VM to know the size of value type.
+ GenTree* temp = fgInsertCommaFormTemp(&retNode->gtOp.gtOp1, info.compMethodInfo->args.retTypeClass);
+
+- GenTree* lclVar = retNode->gtOp.gtOp1->gtOp.gtOp2;
++ GenTree* lclVar = retNode->gtOp.gtOp1->gtOp.gtOp2;
+
+ // The return can't handle all of the trees that could be on the right-hand-side of an assignment,
+ // especially in the case of a struct. Therefore, we need to propagate GTF_DONT_CSE.
+- // If we don't, assertion propagation may, e.g., change a return of a local to a return of "CNS_INT struct 0",
++ // If we don't, assertion propagation may, e.g., change a return of a local to a return of "CNS_INT struct
++ // 0",
+ // which downstream phases can't handle.
+ lclVar->gtFlags |= (retExpr->gtFlags & GTF_DONT_CSE);
+ retNode->gtOp.gtOp1->gtOp.gtOp2 = gtNewOperNode(GT_COMMA, retExpr->TypeGet(), tree, lclVar);
+ }
+ else
+ {
+ // Insert this immediately before the GT_RETURN
+ fgInsertStmtNearEnd(block, tree);
+ }
+ }
+ else
+ {
+ fgInsertStmtAtEnd(block, tree);
+ }
+
+ return tree;
+ }
+
+ // Convert a BBJ_RETURN block in a synchronized method to a BBJ_ALWAYS.
+ // We've previously added a 'try' block around the original program code using fgAddSyncMethodEnterExit().
// in turn passes it to VM to know the size of value type.
GenTree* temp = fgInsertCommaFormTemp(&retNode->gtOp.gtOp1, info.compMethodInfo->args.retTypeClass);
- GenTree* lclVar = retNode->gtOp.gtOp1->gtOp.gtOp2;
+ GenTree* lclVar = retNode->gtOp.gtOp1->gtOp.gtOp2;
+
+ // The return can't handle all of the trees that could be on the right-hand-side of an assignment,
+ // especially in the case of a struct. Therefore, we need to propagate GTF_DONT_CSE.
+ // If we don't, assertion propagation may, e.g., change a return of a local to a return of "CNS_INT struct
+ // 0",
+ // which downstream phases can't handle.
+ lclVar->gtFlags |= (retExpr->gtFlags & GTF_DONT_CSE);
retNode->gtOp.gtOp1->gtOp.gtOp2 = gtNewOperNode(GT_COMMA, retExpr->TypeGet(), tree, lclVar);
}
else
--- /dev/null
+// 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;
+using System.Runtime.CompilerServices;
+using System.Collections.Generic;
+
+public class Test
+{
+ public static int Main()
+ {
+ Test test = new Test();
+ test.GetPair();
+ return 100;
+ }
+
+ [MethodImpl(MethodImplOptions.Synchronized | MethodImplOptions.NoInlining)]
+ internal KeyValuePair<uint, float>? GetPair()
+ {
+ KeyValuePair<uint,float>? result = new KeyValuePair<uint,float>?();
+ return result;
+ }
+}
--- /dev/null
+<?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>
+ <SchemaVersion>2.0</SchemaVersion>
+ <ProjectGuid>{2649FAFE-07BF-4F93-8120-BA9A69285ABB}</ProjectGuid>
+ <OutputType>Exe</OutputType>
+ <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+ <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+ </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>None</DebugType>
+ <Optimize>True</Optimize>
+ </PropertyGroup>
+ <ItemGroup>
+ <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+ <Visible>False</Visible>
+ </CodeAnalysisDependentAssemblyPaths>
+ </ItemGroup>
+ <ItemGroup>
+ <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+ </ItemGroup>
+ <ItemGroup>
+ <Compile Include="$(MSBuildProjectName).cs" />
+ </ItemGroup>
+ <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+ <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>
\ No newline at end of file
projects / platform / upstream / dotnet / runtime.git / commitdiff