Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle additional multireg args #43870

Merged
merged 3 commits into from
Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 56 additions & 22 deletions src/coreclr/src/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo)
if (cSlots == 2)
{
varDsc->SetOtherArgReg(genMapRegArgNumToRegNum(firstAllocatedRegArgNum + 1, TYP_I_IMPL));
varDsc->lvIsMultiRegArg = true;
}
}
#elif defined(UNIX_AMD64_ABI)
Expand All @@ -876,6 +877,7 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo)
{
secondEightByteType = GetEightByteType(structDesc, 1);
secondAllocatedRegArgNum = varDscInfo->allocRegArg(secondEightByteType, 1);
varDsc->lvIsMultiRegArg = true;
}

if (secondEightByteType != TYP_UNDEF)
Expand Down Expand Up @@ -1860,29 +1862,45 @@ bool Compiler::StructPromotionHelper::CanPromoteStructVar(unsigned lclNum)
{
canPromote = false;
}
#if defined(FEATURE_SIMD) && defined(TARGET_ARMARCH)
else if (fieldCnt > 1)
#if defined(TARGET_ARMARCH)
else
{
// If we have a register-passed struct with mixed non-opaque SIMD types (i.e. with defined fields)
// and non-SIMD types, we don't currently handle that case in the prolog, so we can't promote.
for (unsigned i = 0; canPromote && (i < fieldCnt); i++)
{
if (varTypeIsStruct(structPromotionInfo.fields[i].fldType) &&
!compiler->isOpaqueSIMDType(structPromotionInfo.fields[i].fldTypeHnd))
var_types fieldType = structPromotionInfo.fields[i].fldType;
// Non-HFA structs are always passed in general purpose registers.
// If there are any floating point fields, don't promote for now.
// TODO-1stClassStructs: add support in Lowering and prolog generation
// to enable promoting these types.
if (varDsc->lvIsParam && !varDsc->lvIsHfa() && varTypeUsesFloatReg(fieldType))
{
canPromote = false;
}
#if defined(FEATURE_SIMD)
// If we have a register-passed struct with mixed non-opaque SIMD types (i.e. with defined fields)
// and non-SIMD types, we don't currently handle that case in the prolog, so we can't promote.
else if ((fieldCnt > 1) && varTypeIsStruct(fieldType) &&
!compiler->isOpaqueSIMDType(structPromotionInfo.fields[i].fldTypeHnd))
{
canPromote = false;
}
#endif // FEATURE_SIMD
}
}
#elif defined(UNIX_AMD64_ABI)
else
{
SortStructFields();
// Only promote if the field types match the registers.
// Only promote if the field types match the registers, unless we have a single SIMD field.
SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;
compiler->eeGetSystemVAmd64PassStructInRegisterDescriptor(typeHnd, &structDesc);
unsigned regCount = structDesc.eightByteCount;
if (structPromotionInfo.fieldCnt != regCount)
if ((structPromotionInfo.fieldCnt == 1) && varTypeIsSIMD(structPromotionInfo.fields[0].fldType))
{
// Allow the case of promoting a single SIMD field, even if there are multiple registers.
// We will fix this up in the prolog.
}
else if (structPromotionInfo.fieldCnt != regCount)
{
canPromote = false;
}
Expand Down Expand Up @@ -1983,16 +2001,21 @@ bool Compiler::StructPromotionHelper::ShouldPromoteStructVar(unsigned lclNum)
#if FEATURE_MULTIREG_STRUCT_PROMOTE
// Is this a variable holding a value with exactly two fields passed in
// multiple registers?
if ((structPromotionInfo.fieldCnt != 2) && compiler->lvaIsMultiregStruct(varDsc, compiler->info.compIsVarArgs))
if (compiler->lvaIsMultiregStruct(varDsc, compiler->info.compIsVarArgs))
{
JITDUMP("Not promoting multireg struct local V%02u, because lvIsParam is true and #fields != 2\n", lclNum);
shouldPromote = false;
if ((structPromotionInfo.fieldCnt != 2) &&
!((structPromotionInfo.fieldCnt == 1) && varTypeIsSIMD(structPromotionInfo.fields[0].fldType)))
{
JITDUMP("Not promoting multireg struct local V%02u, because lvIsParam is true, #fields != 2 and it's "
"not a single SIMD.\n",
lclNum);
shouldPromote = false;
}
}
else
#endif // !FEATURE_MULTIREG_STRUCT_PROMOTE

// TODO-PERF - Implement struct promotion for incoming multireg structs
// Currently it hits assert(lvFieldCnt==1) in lclvar.cpp line 4417
// TODO-PERF - Implement struct promotion for incoming single-register structs.
// Also the implementation of jmp uses the 4 byte move to store
// byte parameters to the stack, so that if we have a byte field
// with something else occupying the same 4-byte slot, it will
Expand Down Expand Up @@ -2303,16 +2326,28 @@ void Compiler::StructPromotionHelper::PromoteStructVar(unsigned lclNum)
else
#endif // UNIX_AMD64_ABI
{
// TODO: Need to determine if/how to handle split args.
// TODO: Assert that regs are always sequential.
unsigned regIncrement = fieldVarDsc->lvFldOrdinal;
#ifdef TARGET_ARM
if (varDsc->lvIsHfa() && (varDsc->GetHfaType() == TYP_DOUBLE))
regNumber fieldRegNum;
if (index == 0)
{
regIncrement = (fieldVarDsc->lvFldOrdinal * 2);
fieldRegNum = parentArgReg;
}
else if (varDsc->lvIsHfa())
{
unsigned regIncrement = fieldVarDsc->lvFldOrdinal;
#ifdef TARGET_ARM
// TODO: Need to determine if/how to handle split args.
if (varDsc->GetHfaType() == TYP_DOUBLE)
{
regIncrement *= 2;
}
#endif // TARGET_ARM
regNumber fieldRegNum = (regNumber)(parentArgReg + regIncrement);
fieldRegNum = (regNumber)(parentArgReg + regIncrement);
}
else
{
assert(index == 1);
fieldRegNum = varDsc->GetOtherArgReg();
}
fieldVarDsc->SetArgReg(fieldRegNum);
}
}
Expand Down Expand Up @@ -5474,8 +5509,7 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum,
for (unsigned i = 0; i < varDsc->lvFieldCnt; i++)
{
LclVarDsc* fieldVarDsc = lvaGetDesc(firstFieldNum + i);
fieldVarDsc->SetStackOffset(offset);
offset += fieldVarDsc->lvFldOffset;
fieldVarDsc->SetStackOffset(offset + fieldVarDsc->lvFldOffset);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4631,7 +4631,8 @@ GenTree* Compiler::fgMorphMultiregStructArg(GenTree* arg, fgArgTabEntry* fgEntry
var_types loType = loVarDsc->lvType;
var_types hiType = hiVarDsc->lvType;

if (varTypeIsFloating(loType) || varTypeIsFloating(hiType))
if ((varTypeIsFloating(loType) != genIsValidFloatReg(fgEntryPtr->GetRegNum(0))) ||
(varTypeIsFloating(hiType) != genIsValidFloatReg(fgEntryPtr->GetRegNum(1))))
{
// TODO-LSRA - It currently doesn't support the passing of floating point LCL_VARS in the integer
// registers. So for now we will use GT_LCLFLD's to pass this struct (it won't be enregistered)
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/jit/target.h
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,6 @@ typedef unsigned char regNumberSmall;
#endif
#define FEATURE_FIXED_OUT_ARGS 1 // Preallocate the outgoing arg area in the prolog
#define FEATURE_STRUCTPROMOTE 1 // JIT Optimization to promote fields of structs into registers
#define FEATURE_MULTIREG_STRUCT_PROMOTE 0 // True when we want to promote fields of a multireg struct into registers
#define FEATURE_FASTTAILCALL 1 // Tail calls made as epilog+jmp
#define FEATURE_TAILCALL_OPT 1 // opportunistic Tail calls (i.e. without ".tail" prefix) made as fast tail calls.
#define FEATURE_SET_FLAGS 0 // Set to true to force the JIT to mark the trees with GTF_SET_FLAGS when the flags need to be set
Expand All @@ -518,6 +517,7 @@ typedef unsigned char regNumberSmall;
#define FEATURE_MULTIREG_ARGS_OR_RET 1 // Support for passing and/or returning single values in more than one register
#define FEATURE_MULTIREG_ARGS 1 // Support for passing a single argument in more than one register
#define FEATURE_MULTIREG_RET 1 // Support for returning a single value in more than one register
#define FEATURE_MULTIREG_STRUCT_PROMOTE 1 // True when we want to promote fields of a multireg struct into registers
#define FEATURE_STRUCT_CLASSIFIER 1 // Uses a classifier function to determine if structs are passed/returned in more than one register
#define MAX_PASS_MULTIREG_BYTES 32 // Maximum size of a struct that could be passed in more than one register (Max is two SIMD16s)
#define MAX_RET_MULTIREG_BYTES 32 // Maximum size of a struct that could be returned in more than one register (Max is two SIMD16s)
Expand All @@ -531,6 +531,7 @@ typedef unsigned char regNumberSmall;
#define FEATURE_MULTIREG_ARGS_OR_RET 0 // Support for passing and/or returning single values in more than one register
#define FEATURE_MULTIREG_ARGS 0 // Support for passing a single argument in more than one register
#define FEATURE_MULTIREG_RET 0 // Support for returning a single value in more than one register
#define FEATURE_MULTIREG_STRUCT_PROMOTE 0 // True when we want to promote fields of a multireg struct into registers
#define MAX_PASS_MULTIREG_BYTES 0 // No multireg arguments
#define MAX_RET_MULTIREG_BYTES 0 // No multireg return values
#define MAX_ARG_REG_COUNT 1 // Maximum registers used to pass a single argument (no arguments are passed using multiple registers)
Expand Down
40 changes: 40 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_13417/Runtime_13417.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// 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.Runtime.InteropServices;

public class BoundsCheck
{
public static int Main()
{
ReadOnlySpan<byte> span = new ReadOnlySpan<byte>(new byte[7]);
return (int)GetKey(span) + 100;
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static ulong GetKey(ReadOnlySpan<byte> propertyName)
{
const int BitsInByte = 8;
ulong key = 0;
int length = propertyName.Length;

if (length > 3)
{
key = MemoryMarshal.Read<uint>(propertyName);

if (length == 7)
{
key |= (ulong)propertyName[6] << (6 * BitsInByte)
| (ulong)propertyName[5] << (5 * BitsInByte)
| (ulong)propertyName[4] << (4 * BitsInByte)
| (ulong)7 << (7 * BitsInByte);
}
}

return key;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<PropertyGroup>
<DebugType />
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>
47 changes: 47 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_13669/Runtime_13669.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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;

class Program
{
static int Main(string[] args)
{
int result = 0;

ReadOnlySpan<char> span = string.Empty.AsSpan();
for (int i = 0; i < 1_000; i++)
{
result ^= TrimSourceCopied(span).Length;
}

return (result == 0) ? 100 : -1;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static ReadOnlySpan<char> TrimSourceCopied(ReadOnlySpan<char> span)
{
int start = 0;
for (; start < span.Length; start++)
{
if (!char.IsWhiteSpace(span[start]))
{
break;
}
}

int end = span.Length - 1;
for (; end > start; end--)
{
if (!char.IsWhiteSpace(span[end]))
{
break;
}
}

return span.Slice(start, end - start + 1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<PropertyGroup>
<DebugType />
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>