Skip to content

Commit 3ff6056

Browse files
grendellogrendello
committed
Merge branch 'main' into dev/grendel/android-clr-host-local
* 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.
2 parents 74220ed + d9b7515 commit 3ff6056

File tree

65 files changed

+304
-223
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+304
-223
lines changed

‎src/coreclr/jit/compiler.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3790,7 +3790,7 @@ class Compiler
37903790
bool ignoreRoot = false);
37913791

37923792
bool gtSplitTree(
3793-
BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitPointUse);
3793+
BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitPointUse, bool early = false);
37943794

37953795
bool gtStoreDefinesField(
37963796
LclVarDsc* fieldVarDsc, ssize_t offset, unsigned size, ssize_t* pFieldStoreOffset, unsigned* pFieldStoreSize);
@@ -5114,6 +5114,8 @@ class Compiler
51145114
SpillCliqueSucc
51155115
};
51165116

5117+
friend class SubstitutePlaceholdersAndDevirtualizeWalker;
5118+
51175119
// Abstract class for receiving a callback while walking a spill clique
51185120
class SpillCliqueWalker
51195121
{

‎src/coreclr/jit/fginline.cpp

Lines changed: 84 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,9 @@ bool Compiler::TypeInstantiationComplexityExceeds(CORINFO_CLASS_HANDLE handle, i
204204

205205
class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<SubstitutePlaceholdersAndDevirtualizeWalker>
206206
{
207-
bool m_madeChanges = false;
207+
bool m_madeChanges = false;
208+
Statement* m_curStmt = nullptr;
209+
Statement* m_firstNewStmt = nullptr;
208210

209211
public:
210212
enum
@@ -219,11 +221,29 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
219221
{
220222
}
221223

222-
bool MadeChanges()
224+
bool MadeChanges() const
223225
{
224226
return m_madeChanges;
225227
}
226228

229+
// ------------------------------------------------------------------------
230+
// WalkStatement: Walk the tree of a statement, and return the first newly
231+
// added statement if any, otherwise return the original statement.
232+
//
233+
// Arguments:
234+
// stmt - the statement to walk.
235+
//
236+
// Return Value:
237+
// The first newly added statement if any, or the original statement.
238+
//
239+
Statement* WalkStatement(Statement* stmt)
240+
{
241+
m_curStmt = stmt;
242+
m_firstNewStmt = nullptr;
243+
WalkTree(m_curStmt->GetRootNodePointer(), nullptr);
244+
return m_firstNewStmt == nullptr ? m_curStmt : m_firstNewStmt;
245+
}
246+
227247
fgWalkResult PreOrderVisit(GenTree** use, GenTree* user)
228248
{
229249
GenTree* tree = *use;
@@ -586,8 +606,59 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
586606
}
587607

588608
CORINFO_CONTEXT_HANDLE contextInput = context;
609+
context = nullptr;
589610
m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context,
590611
isLateDevirtualization, explicitTailCall);
612+
613+
if (!call->IsVirtual())
614+
{
615+
assert(context != nullptr);
616+
CORINFO_CALL_INFO callInfo = {};
617+
callInfo.hMethod = method;
618+
callInfo.methodFlags = methodFlags;
619+
m_compiler->impMarkInlineCandidate(call, context, false, &callInfo);
620+
621+
if (call->IsInlineCandidate())
622+
{
623+
Statement* newStmt = nullptr;
624+
GenTree** callUse = nullptr;
625+
if (m_compiler->gtSplitTree(m_compiler->compCurBB, m_curStmt, call, &newStmt, &callUse, true))
626+
{
627+
if (m_firstNewStmt == nullptr)
628+
{
629+
m_firstNewStmt = newStmt;
630+
}
631+
}
632+
633+
// If the call is the root expression in a statement, and it returns void,
634+
// we can inline it directly without creating a RET_EXPR.
635+
if (parent != nullptr || call->gtReturnType != TYP_VOID)
636+
{
637+
Statement* stmt = m_compiler->gtNewStmt(call);
638+
m_compiler->fgInsertStmtBefore(m_compiler->compCurBB, m_curStmt, stmt);
639+
if (m_firstNewStmt == nullptr)
640+
{
641+
m_firstNewStmt = stmt;
642+
}
643+
644+
GenTreeRetExpr* retExpr =
645+
m_compiler->gtNewInlineCandidateReturnExpr(call->AsCall(),
646+
genActualType(call->TypeGet()));
647+
call->GetSingleInlineCandidateInfo()->retExpr = retExpr;
648+
649+
JITDUMP("Creating new RET_EXPR for [%06u]:\n", call->gtTreeID);
650+
DISPTREE(retExpr);
651+
652+
*pTree = retExpr;
653+
}
654+
655+
call->GetSingleInlineCandidateInfo()->exactContextHandle = context;
656+
INDEBUG(call->GetSingleInlineCandidateInfo()->inlinersContext = call->gtInlineContext);
657+
658+
JITDUMP("New inline candidate due to late devirtualization:\n");
659+
DISPTREE(call);
660+
}
661+
}
591662
m_madeChanges = true;
592663
}
593664
}
@@ -730,17 +801,10 @@ PhaseStatus Compiler::fgInline()
730801
do
731802
{
732803
// Make the current basic block address available globally
733-
compCurBB = block;
734-
735-
for (Statement* const stmt : block->Statements())
804+
compCurBB = block;
805+
Statement* stmt = block->firstStmt();
806+
while (stmt != nullptr)
736807
{
737-
738-
#if defined(DEBUG)
739-
// In debug builds we want the inline tree to show all failed
740-
// inlines. Some inlines may fail very early and never make it to
741-
// candidate stage. So scan the tree looking for those early failures.
742-
fgWalkTreePre(stmt->GetRootNodePointer(), fgFindNonInlineCandidate, stmt);
743-
#endif
744808
// See if we need to replace some return value place holders.
745809
// Also, see if this replacement enables further devirtualization.
746810
//
@@ -755,7 +819,7 @@ PhaseStatus Compiler::fgInline()
755819
// possible further optimization, as the (now complete) GT_RET_EXPR
756820
// replacement may have enabled optimizations by providing more
757821
// specific types for trees or variables.
758-
walker.WalkTree(stmt->GetRootNodePointer(), nullptr);
822+
stmt = walker.WalkStatement(stmt);
759823

760824
GenTree* expr = stmt->GetRootNode();
761825

@@ -805,6 +869,13 @@ PhaseStatus Compiler::fgInline()
805869
madeChanges = true;
806870
stmt->SetRootNode(expr->AsOp()->gtOp1);
807871
}
872+
873+
#if defined(DEBUG)
874+
// In debug builds we want the inline tree to show all failed
875+
// inlines.
876+
fgWalkTreePre(stmt->GetRootNodePointer(), fgFindNonInlineCandidate, stmt);
877+
#endif
878+
stmt = stmt->GetNextStmt();
808879
}
809880

810881
block = block->Next();

‎src/coreclr/jit/gentree.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17005,6 +17005,8 @@ bool Compiler::gtTreeHasSideEffects(GenTree* tree, GenTreeFlags flags /* = GTF_S
1700517005
// firstNewStmt - [out] The first new statement that was introduced.
1700617006
// [firstNewStmt..stmt) are the statements added by this function.
1700717007
// splitNodeUse - The use of the tree to split at.
17008+
// early - The run is in the early phase where we still don't have valid
17009+
// GTF_GLOB_REF yet.
1700817010
//
1700917011
// Notes:
1701017012
// This method turns all non-invariant nodes that would be executed before
@@ -17025,14 +17027,19 @@ bool Compiler::gtTreeHasSideEffects(GenTree* tree, GenTreeFlags flags /* = GTF_S
1702517027
// Returns:
1702617028
// True if any changes were made; false if nothing needed to be done to split the tree.
1702717029
//
17028-
bool Compiler::gtSplitTree(
17029-
BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitNodeUse)
17030+
bool Compiler::gtSplitTree(BasicBlock* block,
17031+
Statement* stmt,
17032+
GenTree* splitPoint,
17033+
Statement** firstNewStmt,
17034+
GenTree*** splitNodeUse,
17035+
bool early)
1703017036
{
1703117037
class Splitter final : public GenTreeVisitor<Splitter>
1703217038
{
1703317039
BasicBlock* m_bb;
1703417040
Statement* m_splitStmt;
1703517041
GenTree* m_splitNode;
17042+
bool m_early;
1703617043

1703717044
struct UseInfo
1703817045
{
@@ -17049,11 +17056,12 @@ bool Compiler::gtSplitTree(
1704917056
UseExecutionOrder = true
1705017057
};
1705117058

17052-
Splitter(Compiler* compiler, BasicBlock* bb, Statement* stmt, GenTree* splitNode)
17059+
Splitter(Compiler* compiler, BasicBlock* bb, Statement* stmt, GenTree* splitNode, bool early)
1705317060
: GenTreeVisitor(compiler)
1705417061
, m_bb(bb)
1705517062
, m_splitStmt(stmt)
1705617063
, m_splitNode(splitNode)
17064+
, m_early(early)
1705717065
, m_useStack(compiler->getAllocator(CMK_ArrayStack))
1705817066
{
1705917067
}
@@ -17195,7 +17203,8 @@ bool Compiler::gtSplitTree(
1719517203
return;
1719617204
}
1719717205

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

17281-
Splitter splitter(this, block, stmt, splitPoint);
17290+
Splitter splitter(this, block, stmt, splitPoint, early);
1728217291
splitter.WalkTree(stmt->GetRootNodePointer(), nullptr);
1728317292
*firstNewStmt = splitter.FirstStatement;
1728417293
*splitNodeUse = splitter.SplitNodeUse;

‎src/coreclr/jit/importercalls.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
329329
assert((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG &&
330330
(sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_NATIVEVARARG);
331331

332-
call = gtNewIndCallNode(stubAddr, callRetTyp);
332+
call = gtNewIndCallNode(stubAddr, callRetTyp, di);
333333

334334
call->gtFlags |= GTF_EXCEPT | (stubAddr->gtFlags & GTF_GLOB_EFFECT);
335335
call->gtFlags |= GTF_CALL_VIRT_STUB;

‎src/libraries/Microsoft.Bcl.Cryptography/src/System/Security/Cryptography/NetStandardShims.cs

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -100,25 +100,14 @@ internal static bool TryGetHashAndReset(
100100
Span<byte> destination,
101101
out int bytesWritten)
102102
{
103-
int hashSize = hash.AlgorithmName.Name switch
104-
{
105-
nameof(HashAlgorithmName.MD5) => 128 >> 3,
106-
nameof(HashAlgorithmName.SHA1) => 160 >> 3,
107-
nameof(HashAlgorithmName.SHA256) => 256 >> 3,
108-
nameof(HashAlgorithmName.SHA384) => 384 >> 3,
109-
nameof(HashAlgorithmName.SHA512) => 512 >> 3,
110-
_ => throw new CryptographicException(),
111-
};
112-
113-
if (destination.Length < hashSize)
103+
byte[] actual = hash.GetHashAndReset();
104+
105+
if (destination.Length < actual.Length)
114106
{
115107
bytesWritten = 0;
116108
return false;
117109
}
118110

119-
byte[] actual = hash.GetHashAndReset();
120-
Debug.Assert(actual.Length == hashSize);
121-
122111
actual.AsSpan().CopyTo(destination);
123112
bytesWritten = actual.Length;
124113
return true;

‎src/libraries/System.Private.CoreLib/src/System/Buffers/Text/FormattingHelpers.CountDigits.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public static int CountDigits(ulong value)
2525
Debug.Assert(log2ToPow10.Length == 64);
2626

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

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

5858
// Return the number of digits based on the power of 10, shifted by 1
5959
// if it falls below the threshold.
60-
bool lessThan = value < powerOf10;
61-
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
60+
int index = (int)elementOffset;
61+
return index - (value < powerOf10 ? 1 : 0);
6262
}
6363

6464
[MethodImpl(MethodImplOptions.AggressiveInlining)]

‎src/libraries/System.Private.CoreLib/src/System/Half.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ public static explicit operator Half(float value)
746746
// Extract sign bit
747747
uint sign = (bitValue & float.SignMask) >> 16;
748748
// Detecting NaN (~0u if a is not NaN)
749-
uint realMask = (uint)(Unsafe.BitCast<bool, sbyte>(float.IsNaN(value)) - 1);
749+
uint realMask = float.IsNaN(value) ? 0u : ~0u;
750750
// Clear sign bit
751751
value = float.Abs(value);
752752
// Rectify values that are Infinity in Half. (float.Min now emits vminps instruction if one of two arguments is a constant)
@@ -1075,17 +1075,15 @@ public static explicit operator float(Half value)
10751075
// Extract exponent bits of value (BiasedExponent is not for here as it performs unnecessary shift)
10761076
uint offsetExponent = bitValueInProcess & HalfExponentMask;
10771077
// ~0u when value is subnormal, 0 otherwise
1078-
uint subnormalMask = (uint)-Unsafe.BitCast<bool, byte>(offsetExponent == 0u);
1079-
// ~0u when value is either Infinity or NaN, 0 otherwise
1080-
int infinityOrNaNMask = Unsafe.BitCast<bool, byte>(offsetExponent == HalfExponentMask);
1078+
uint subnormalMask = offsetExponent == 0u ? ~0u : 0u;
10811079
// 0x3880_0000u if value is subnormal, 0 otherwise
10821080
uint maskedExponentLowerBound = subnormalMask & ExponentLowerBound;
10831081
// 0x3880_0000u if value is subnormal, 0x3800_0000u otherwise
10841082
uint offsetMaskedExponentLowerBound = ExponentOffset | maskedExponentLowerBound;
10851083
// Match the position of the boundary of exponent bits and fraction bits with IEEE 754 Binary32(Single)
10861084
bitValueInProcess <<= 13;
10871085
// Double the offsetMaskedExponentLowerBound if value is either Infinity or NaN
1088-
offsetMaskedExponentLowerBound <<= infinityOrNaNMask;
1086+
offsetMaskedExponentLowerBound <<= offsetExponent == HalfExponentMask ? 1 : 0;
10891087
// Extract exponent bits and fraction bits of value
10901088
bitValueInProcess &= HalfToSingleBitsMask;
10911089
// Adjust exponent to match the range of exponent

‎src/native/managed/cdacreader/Directory.Build.props

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<Project>
2+
<Import Project="..\Directory.Build.props" />
3+
<ItemGroup>
4+
<AssemblyAttribute Include="System.Diagnostics.CodeAnalysis.ExperimentalAttribute">
5+
<_Parameter1>NETCDAC0001</_Parameter1>
6+
</AssemblyAttribute>
7+
</ItemGroup>
8+
</Project>

‎src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Microsoft.Diagnostics.DataContractReader;
99
/// <summary>
1010
/// A registry of all the contracts that may be provided by a target.
1111
/// </summary>
12-
internal abstract class ContractRegistry
12+
public abstract class ContractRegistry
1313
{
1414
/// <summary>
1515
/// Gets an instance of the Exception contract for the target.

‎src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/Extensions/ICodeVersionsExtensions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33

44
namespace Microsoft.Diagnostics.DataContractReader.Contracts.Extensions;
55

6-
internal static class ICodeVersionsExtensions
6+
public static class ICodeVersionsExtensions
77
{
8-
internal static NativeCodeVersionHandle GetActiveNativeCodeVersion(this ICodeVersions cv, TargetPointer methodDesc)
8+
public static NativeCodeVersionHandle GetActiveNativeCodeVersion(this ICodeVersions cv, TargetPointer methodDesc)
99
{
1010
ILCodeVersionHandle ilCodeVersionHandle = cv.GetActiveILCodeVersion(methodDesc);
1111
return cv.GetActiveNativeCodeVersionForILCodeVersion(methodDesc, ilCodeVersionHandle);

‎src/native/managed/cdacreader/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/Extensions/IReJITExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
namespace Microsoft.Diagnostics.DataContractReader.Contracts.Extensions;
77

8-
internal static class IReJITExtensions
8+
public static class IReJITExtensions
99
{
1010
public static IEnumerable<TargetNUInt> GetRejitIds(this IReJIT rejit, Target target, TargetPointer methodDesc)
1111
{

0 commit comments

Comments
 (0)