Skip to content

Commit

Permalink
JIT: Fix indir flags propagation for a couple of cases (#79751)
Browse files Browse the repository at this point in the history
SetIndirExceptionFlags expects only unary indirs (reads) and does not
handle other cases correctly. Add an assert for it and fix the users.

Only fgMorphStoreDynBlock had the bug since gtUpdateNodeOperSideEffects
assumes the caller will propagate effect flags from operands afterwards.

Extracted from early liveness PR.

Fix #79750
  • Loading branch information
jakobbotsch authored Dec 17, 2022
1 parent 48edd27 commit e66a6a3
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 3 deletions.
7 changes: 5 additions & 2 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9234,7 +9234,7 @@ void Compiler::gtUpdateNodeOperSideEffects(GenTree* tree)
tree->gtFlags &= ~GTF_EXCEPT;
if (tree->OperIsIndirOrArrMetaData())
{
tree->SetIndirExceptionFlags(this);
tree->gtFlags |= GTF_IND_NONFAULTING;
}
}

Expand Down Expand Up @@ -9981,9 +9981,12 @@ bool GenTree::Precedes(GenTree* other)
// Arguments:
// comp - compiler instance
//
// Remarks:
// This should only be used for reads.
//
void GenTree::SetIndirExceptionFlags(Compiler* comp)
{
assert(OperIsIndirOrArrMetaData());
assert(OperIsIndirOrArrMetaData() && OperIsUnary());

if (OperMayThrow(comp))
{
Expand Down
11 changes: 10 additions & 1 deletion src/coreclr/jit/morphblock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1593,7 +1593,16 @@ GenTree* Compiler::fgMorphStoreDynBlock(GenTreeStoreDynBlk* tree)
}

tree->SetAllEffectsFlags(tree->Addr(), tree->Data(), tree->gtDynamicSize);
tree->SetIndirExceptionFlags(this);

if (tree->OperMayThrow(this))
{
tree->gtFlags |= GTF_EXCEPT;
}
else
{
tree->gtFlags |= GTF_IND_NONFAULTING;
}

tree->gtFlags |= GTF_ASG;

return tree;
Expand Down
23 changes: 23 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_79750/Runtime_79750.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;

class Runtime_79750
{
static int Main(string[] args)
{
byte dest = 0;
byte source = 100;
uint size = GetSize();
Unsafe.CopyBlock(ref dest, ref GetAddr(ref source), size);

return dest;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static ref byte GetAddr(ref byte a) => ref a;

[MethodImpl(MethodImplOptions.NoInlining)]
private static uint GetSize() => 1;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<Optimize>True</Optimize>
<AllowUnsafeBlocks>True</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit e66a6a3

Please sign in to comment.