Skip to content

Commit

Permalink
Merge branch 'main' into dev/grendel/android-clr-host-local
Browse files Browse the repository at this point in the history
* main:
  Make cdac APIs public but experimental (dotnet#111180)
  JIT: Enable inlining for late devirtualization (dotnet#110827)
  Remove unsafe `bool` casts (dotnet#111024)
  Fix shimmed implementation of TryGetHashAndReset to handle HMAC.
  • Loading branch information
grendello committed Jan 29, 2025
2 parents 74220ed + d9b7515 commit 3ff6056
Show file tree
Hide file tree
Showing 65 changed files with 304 additions and 223 deletions.
4 changes: 3 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -3790,7 +3790,7 @@ class Compiler
bool ignoreRoot = false);

bool gtSplitTree(
BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitPointUse);
BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitPointUse, bool early = false);

bool gtStoreDefinesField(
LclVarDsc* fieldVarDsc, ssize_t offset, unsigned size, ssize_t* pFieldStoreOffset, unsigned* pFieldStoreSize);
Expand Down Expand Up @@ -5114,6 +5114,8 @@ class Compiler
SpillCliqueSucc
};

friend class SubstitutePlaceholdersAndDevirtualizeWalker;

// Abstract class for receiving a callback while walking a spill clique
class SpillCliqueWalker
{
Expand Down
97 changes: 84 additions & 13 deletions src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ bool Compiler::TypeInstantiationComplexityExceeds(CORINFO_CLASS_HANDLE handle, i

class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<SubstitutePlaceholdersAndDevirtualizeWalker>
{
bool m_madeChanges = false;
bool m_madeChanges = false;
Statement* m_curStmt = nullptr;
Statement* m_firstNewStmt = nullptr;

public:
enum
Expand All @@ -219,11 +221,29 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
{
}

bool MadeChanges()
bool MadeChanges() const
{
return m_madeChanges;
}

// ------------------------------------------------------------------------
// WalkStatement: Walk the tree of a statement, and return the first newly
// added statement if any, otherwise return the original statement.
//
// Arguments:
// stmt - the statement to walk.
//
// Return Value:
// The first newly added statement if any, or the original statement.
//
Statement* WalkStatement(Statement* stmt)
{
m_curStmt = stmt;
m_firstNewStmt = nullptr;
WalkTree(m_curStmt->GetRootNodePointer(), nullptr);
return m_firstNewStmt == nullptr ? m_curStmt : m_firstNewStmt;
}

fgWalkResult PreOrderVisit(GenTree** use, GenTree* user)
{
GenTree* tree = *use;
Expand Down Expand Up @@ -586,8 +606,59 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
}

CORINFO_CONTEXT_HANDLE contextInput = context;
context = nullptr;
m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context,
isLateDevirtualization, explicitTailCall);

if (!call->IsVirtual())
{
assert(context != nullptr);
CORINFO_CALL_INFO callInfo = {};
callInfo.hMethod = method;
callInfo.methodFlags = methodFlags;
m_compiler->impMarkInlineCandidate(call, context, false, &callInfo);

if (call->IsInlineCandidate())
{
Statement* newStmt = nullptr;
GenTree** callUse = nullptr;
if (m_compiler->gtSplitTree(m_compiler->compCurBB, m_curStmt, call, &newStmt, &callUse, true))
{
if (m_firstNewStmt == nullptr)
{
m_firstNewStmt = newStmt;
}
}

// If the call is the root expression in a statement, and it returns void,
// we can inline it directly without creating a RET_EXPR.
if (parent != nullptr || call->gtReturnType != TYP_VOID)
{
Statement* stmt = m_compiler->gtNewStmt(call);
m_compiler->fgInsertStmtBefore(m_compiler->compCurBB, m_curStmt, stmt);
if (m_firstNewStmt == nullptr)
{
m_firstNewStmt = stmt;
}

GenTreeRetExpr* retExpr =
m_compiler->gtNewInlineCandidateReturnExpr(call->AsCall(),
genActualType(call->TypeGet()));
call->GetSingleInlineCandidateInfo()->retExpr = retExpr;

JITDUMP("Creating new RET_EXPR for [%06u]:\n", call->gtTreeID);
DISPTREE(retExpr);

*pTree = retExpr;
}

call->GetSingleInlineCandidateInfo()->exactContextHandle = context;
INDEBUG(call->GetSingleInlineCandidateInfo()->inlinersContext = call->gtInlineContext);

JITDUMP("New inline candidate due to late devirtualization:\n");
DISPTREE(call);
}
}
m_madeChanges = true;
}
}
Expand Down Expand Up @@ -730,17 +801,10 @@ PhaseStatus Compiler::fgInline()
do
{
// Make the current basic block address available globally
compCurBB = block;

for (Statement* const stmt : block->Statements())
compCurBB = block;
Statement* stmt = block->firstStmt();
while (stmt != nullptr)
{

#if defined(DEBUG)
// In debug builds we want the inline tree to show all failed
// inlines. Some inlines may fail very early and never make it to
// candidate stage. So scan the tree looking for those early failures.
fgWalkTreePre(stmt->GetRootNodePointer(), fgFindNonInlineCandidate, stmt);
#endif
// See if we need to replace some return value place holders.
// Also, see if this replacement enables further devirtualization.
//
Expand All @@ -755,7 +819,7 @@ PhaseStatus Compiler::fgInline()
// possible further optimization, as the (now complete) GT_RET_EXPR
// replacement may have enabled optimizations by providing more
// specific types for trees or variables.
walker.WalkTree(stmt->GetRootNodePointer(), nullptr);
stmt = walker.WalkStatement(stmt);

GenTree* expr = stmt->GetRootNode();

Expand Down Expand Up @@ -805,6 +869,13 @@ PhaseStatus Compiler::fgInline()
madeChanges = true;
stmt->SetRootNode(expr->AsOp()->gtOp1);
}

#if defined(DEBUG)
// In debug builds we want the inline tree to show all failed
// inlines.
fgWalkTreePre(stmt->GetRootNodePointer(), fgFindNonInlineCandidate, stmt);
#endif
stmt = stmt->GetNextStmt();
}

block = block->Next();
Expand Down
19 changes: 14 additions & 5 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17005,6 +17005,8 @@ bool Compiler::gtTreeHasSideEffects(GenTree* tree, GenTreeFlags flags /* = GTF_S
// firstNewStmt - [out] The first new statement that was introduced.
// [firstNewStmt..stmt) are the statements added by this function.
// splitNodeUse - The use of the tree to split at.
// early - The run is in the early phase where we still don't have valid
// GTF_GLOB_REF yet.
//
// Notes:
// This method turns all non-invariant nodes that would be executed before
Expand All @@ -17025,14 +17027,19 @@ bool Compiler::gtTreeHasSideEffects(GenTree* tree, GenTreeFlags flags /* = GTF_S
// Returns:
// True if any changes were made; false if nothing needed to be done to split the tree.
//
bool Compiler::gtSplitTree(
BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitNodeUse)
bool Compiler::gtSplitTree(BasicBlock* block,
Statement* stmt,
GenTree* splitPoint,
Statement** firstNewStmt,
GenTree*** splitNodeUse,
bool early)
{
class Splitter final : public GenTreeVisitor<Splitter>
{
BasicBlock* m_bb;
Statement* m_splitStmt;
GenTree* m_splitNode;
bool m_early;

struct UseInfo
{
Expand All @@ -17049,11 +17056,12 @@ bool Compiler::gtSplitTree(
UseExecutionOrder = true
};

Splitter(Compiler* compiler, BasicBlock* bb, Statement* stmt, GenTree* splitNode)
Splitter(Compiler* compiler, BasicBlock* bb, Statement* stmt, GenTree* splitNode, bool early)
: GenTreeVisitor(compiler)
, m_bb(bb)
, m_splitStmt(stmt)
, m_splitNode(splitNode)
, m_early(early)
, m_useStack(compiler->getAllocator(CMK_ArrayStack))
{
}
Expand Down Expand Up @@ -17195,7 +17203,8 @@ bool Compiler::gtSplitTree(
return;
}

if ((*use)->OperIs(GT_LCL_VAR) && !m_compiler->lvaGetDesc((*use)->AsLclVarCommon())->IsAddressExposed())
if ((*use)->OperIs(GT_LCL_VAR) && !m_compiler->lvaGetDesc((*use)->AsLclVarCommon())->IsAddressExposed() &&
!(m_early && m_compiler->lvaGetDesc((*use)->AsLclVarCommon())->lvHasLdAddrOp))
{
// The splitting we do here should always guarantee that we
// only introduce locals for the tree edges that overlap the
Expand Down Expand Up @@ -17278,7 +17287,7 @@ bool Compiler::gtSplitTree(
}
};

Splitter splitter(this, block, stmt, splitPoint);
Splitter splitter(this, block, stmt, splitPoint, early);
splitter.WalkTree(stmt->GetRootNodePointer(), nullptr);
*firstNewStmt = splitter.FirstStatement;
*splitNodeUse = splitter.SplitNodeUse;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
assert((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG &&
(sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_NATIVEVARARG);

call = gtNewIndCallNode(stubAddr, callRetTyp);
call = gtNewIndCallNode(stubAddr, callRetTyp, di);

call->gtFlags |= GTF_EXCEPT | (stubAddr->gtFlags & GTF_GLOB_EFFECT);
call->gtFlags |= GTF_CALL_VIRT_STUB;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,25 +100,14 @@ internal static bool TryGetHashAndReset(
Span<byte> destination,
out int bytesWritten)
{
int hashSize = hash.AlgorithmName.Name switch
{
nameof(HashAlgorithmName.MD5) => 128 >> 3,
nameof(HashAlgorithmName.SHA1) => 160 >> 3,
nameof(HashAlgorithmName.SHA256) => 256 >> 3,
nameof(HashAlgorithmName.SHA384) => 384 >> 3,
nameof(HashAlgorithmName.SHA512) => 512 >> 3,
_ => throw new CryptographicException(),
};

if (destination.Length < hashSize)
byte[] actual = hash.GetHashAndReset();

if (destination.Length < actual.Length)
{
bytesWritten = 0;
return false;
}

byte[] actual = hash.GetHashAndReset();
Debug.Assert(actual.Length == hashSize);

actual.AsSpan().CopyTo(destination);
bytesWritten = actual.Length;
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public static int CountDigits(ulong value)
Debug.Assert(log2ToPow10.Length == 64);

// TODO: Replace with log2ToPow10[BitOperations.Log2(value)] once https://github.com/dotnet/runtime/issues/79257 is fixed
uint index = Unsafe.Add(ref MemoryMarshal.GetReference(log2ToPow10), BitOperations.Log2(value));
nint elementOffset = Unsafe.Add(ref MemoryMarshal.GetReference(log2ToPow10), BitOperations.Log2(value));

// Read the associated power of 10.
ReadOnlySpan<ulong> powersOf10 =
Expand All @@ -52,13 +52,13 @@ public static int CountDigits(ulong value)
1000000000000000000,
10000000000000000000,
];
Debug.Assert((index + 1) <= powersOf10.Length);
ulong powerOf10 = Unsafe.Add(ref MemoryMarshal.GetReference(powersOf10), index);
Debug.Assert((elementOffset + 1) <= powersOf10.Length);
ulong powerOf10 = Unsafe.Add(ref MemoryMarshal.GetReference(powersOf10), elementOffset);

// Return the number of digits based on the power of 10, shifted by 1
// if it falls below the threshold.
bool lessThan = value < powerOf10;
return (int)(index - Unsafe.As<bool, byte>(ref lessThan)); // while arbitrary bools may be non-0/1, comparison operators are expected to return 0/1
int index = (int)elementOffset;
return index - (value < powerOf10 ? 1 : 0);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down
8 changes: 3 additions & 5 deletions src/libraries/System.Private.CoreLib/src/System/Half.cs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ public static explicit operator Half(float value)
// Extract sign bit
uint sign = (bitValue & float.SignMask) >> 16;
// Detecting NaN (~0u if a is not NaN)
uint realMask = (uint)(Unsafe.BitCast<bool, sbyte>(float.IsNaN(value)) - 1);
uint realMask = float.IsNaN(value) ? 0u : ~0u;
// Clear sign bit
value = float.Abs(value);
// Rectify values that are Infinity in Half. (float.Min now emits vminps instruction if one of two arguments is a constant)
Expand Down Expand Up @@ -1075,17 +1075,15 @@ public static explicit operator float(Half value)
// Extract exponent bits of value (BiasedExponent is not for here as it performs unnecessary shift)
uint offsetExponent = bitValueInProcess & HalfExponentMask;
// ~0u when value is subnormal, 0 otherwise
uint subnormalMask = (uint)-Unsafe.BitCast<bool, byte>(offsetExponent == 0u);
// ~0u when value is either Infinity or NaN, 0 otherwise
int infinityOrNaNMask = Unsafe.BitCast<bool, byte>(offsetExponent == HalfExponentMask);
uint subnormalMask = offsetExponent == 0u ? ~0u : 0u;
// 0x3880_0000u if value is subnormal, 0 otherwise
uint maskedExponentLowerBound = subnormalMask & ExponentLowerBound;
// 0x3880_0000u if value is subnormal, 0x3800_0000u otherwise
uint offsetMaskedExponentLowerBound = ExponentOffset | maskedExponentLowerBound;
// Match the position of the boundary of exponent bits and fraction bits with IEEE 754 Binary32(Single)
bitValueInProcess <<= 13;
// Double the offsetMaskedExponentLowerBound if value is either Infinity or NaN
offsetMaskedExponentLowerBound <<= infinityOrNaNMask;
offsetMaskedExponentLowerBound <<= offsetExponent == HalfExponentMask ? 1 : 0;
// Extract exponent bits and fraction bits of value
bitValueInProcess &= HalfToSingleBitsMask;
// Adjust exponent to match the range of exponent
Expand Down
8 changes: 8 additions & 0 deletions src/native/managed/cdacreader/Directory.Build.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project>
<Import Project="..\Directory.Build.props" />
<ItemGroup>
<AssemblyAttribute Include="System.Diagnostics.CodeAnalysis.ExperimentalAttribute">
<_Parameter1>NETCDAC0001</_Parameter1>
</AssemblyAttribute>
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Microsoft.Diagnostics.DataContractReader;
/// <summary>
/// A registry of all the contracts that may be provided by a target.
/// </summary>
internal abstract class ContractRegistry
public abstract class ContractRegistry
{
/// <summary>
/// Gets an instance of the Exception contract for the target.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

namespace Microsoft.Diagnostics.DataContractReader.Contracts.Extensions;

internal static class ICodeVersionsExtensions
public static class ICodeVersionsExtensions
{
internal static NativeCodeVersionHandle GetActiveNativeCodeVersion(this ICodeVersions cv, TargetPointer methodDesc)
public static NativeCodeVersionHandle GetActiveNativeCodeVersion(this ICodeVersions cv, TargetPointer methodDesc)
{
ILCodeVersionHandle ilCodeVersionHandle = cv.GetActiveILCodeVersion(methodDesc);
return cv.GetActiveNativeCodeVersionForILCodeVersion(methodDesc, ilCodeVersionHandle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace Microsoft.Diagnostics.DataContractReader.Contracts.Extensions;

internal static class IReJITExtensions
public static class IReJITExtensions
{
public static IEnumerable<TargetNUInt> GetRejitIds(this IReJIT rejit, Target target, TargetPointer methodDesc)
{
Expand Down
Loading

0 comments on commit 3ff6056

Please sign in to comment.