Skip to content

Commit

Permalink
JIT: Fix LowerHWIntrinsicGetElement for xarch (#104987)
Browse files Browse the repository at this point in the history
Ensure we don't reorder evaluation and change exceptional behavior.

Closes #89565.

Co-authored-by: Jakob Botsch Nielsen <[email protected]>
  • Loading branch information
AndyAyersMS and jakobbotsch authored Jul 18, 2024
1 parent c95095e commit 2d17105
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 2 deletions.
39 changes: 37 additions & 2 deletions src/coreclr/jit/lowerxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5074,8 +5074,43 @@ GenTree* Lowering::LowerHWIntrinsicGetElement(GenTreeHWIntrinsic* node)
uint32_t newScale;
int32_t newOffset;

GenTreeIndir* indir = op1->AsIndir();
GenTree* addr = indir->Addr();
// Normally we'd evaluate op1 (indir), then op2 (element index).
// We like to be able to reorder these to fold op2 into the indir.

GenTreeIndir* indir = op1->AsIndir();
GenTree* addr = indir->Addr();
bool const canMoveTheIndirLater = IsInvariantInRange(indir, node);

// If we can't move the indir, force evaluation of its side effects.
//
if (!canMoveTheIndirLater)
{
// Force evaluation of the address, if it is complex
//
if (!(addr->IsInvariant() || addr->OperIsLocal()))
{
addr->ClearContained();
LIR::Use addrUse(BlockRange(), &indir->Addr(), indir);
addrUse.ReplaceWithLclVar(comp);
addr = indir->Addr();
}

// If the indir can fault, do a null check.
//
if (indir->OperMayThrow(comp))
{
GenTree* addrClone = comp->gtCloneExpr(addr);
GenTree* nullcheck = comp->gtNewNullCheck(addrClone, comp->compCurBB);
BlockRange().InsertBefore(indir, addrClone, nullcheck);
LowerNode(nullcheck);

indir->gtFlags |= GTF_IND_NONFAULTING;
}

// We should now be able to move the indir
//
indir->gtFlags &= ~GTF_EXCEPT;
}

if (addr->OperIsAddrMode())
{
Expand Down
55 changes: 55 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_89565/Runtime_89565.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;
using System.Runtime.Intrinsics;
using Xunit;

public class Runtime_89565
{
[Fact]
public static unsafe int Test()
{
int result = 0;
try
{
Foo(null);
}
catch (NullReferenceException)
{
result += 50;
}
catch
{

}

try
{
Bar(null, 0);
}
catch (DivideByZeroException)
{
result += 50;
}
catch
{

}

return result;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static unsafe float Foo(Vector256<float>* v)
{
return (*v)[8];
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static unsafe float Bar(Vector256<float>* v, int x)
{
return (*v)[8/x];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 2d17105

Please sign in to comment.