From 10ea4c6d185be26968a7805670242b87f9311ccd Mon Sep 17 00:00:00 2001 From: Mike Danes Date: Fri, 3 Mar 2017 19:23:33 +0200 Subject: [PATCH] Add/improve assertion propagation comments Commit migrated from https://github.com/dotnet/coreclr/commit/f81e625c6a030436e3e50d699e44f529d797997c --- src/coreclr/src/jit/assertionprop.cpp | 6 +++++- src/coreclr/src/jit/compiler.h | 6 ++++++ src/coreclr/src/jit/gentree.h | 1 + src/coreclr/src/jit/valuenum.cpp | 14 ++++++++------ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/coreclr/src/jit/assertionprop.cpp b/src/coreclr/src/jit/assertionprop.cpp index cf7190b..192ed5c 100644 --- a/src/coreclr/src/jit/assertionprop.cpp +++ b/src/coreclr/src/jit/assertionprop.cpp @@ -4471,6 +4471,7 @@ ASSERT_TP* Compiler::optComputeAssertionGen() { if (tree->gtOper == GT_JTRUE) { + // A GT_TRUE is always the last node in a tree, so we can break here assert((tree->gtNext == nullptr) && (stmt->gtNext == nullptr)); jtrue = tree; break; @@ -4498,17 +4499,19 @@ ASSERT_TP* Compiler::optComputeAssertionGen() index &= OAE_INDEX_MASK; // Currently OAE_NEXT_EDGE is only used with OAK_NO_THROW assertions assert(optGetAssertion(index)->assertionKind == OAK_NO_THROW); - // Don't bother with implied/complementary assertions, there aren't any for OAK_NO_THROW + // Don't bother with implied assertions, there aren't any for OAK_NO_THROW BitVecOps::AddElemD(apTraits, valueGen, index - 1); } else { + // If GT_JTRUE, and true path, update jumpDestValueGen. optImpliedAssertions(index, jumpDestValueGen); BitVecOps::AddElemD(apTraits, jumpDestValueGen, index - 1); index = optFindComplementary(index); if (index != NO_ASSERTION_INDEX) { + // If GT_JTRUE, and false path and we have a complementary assertion available update valueGen optImpliedAssertions(index, valueGen); BitVecOps::AddElemD(apTraits, valueGen, index - 1); } @@ -5117,6 +5120,7 @@ void Compiler::optAssertionPropMain() { if (tree->OperIs(GT_JTRUE)) { + // A GT_TRUE is always the last node in a tree, so we can break here assert((tree->gtNext == nullptr) && (stmt->gtNext == nullptr)); break; } diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 2f96b85..be71875 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -5882,6 +5882,12 @@ public: typedef unsigned short AssertionIndex; + // By default the assertion generated by a GT_JTRUE node holds on the true (bbJumpDest) edge. + // If the OAE_NEXT_EDGE bit of the assertion index is set then the assertion holds on the false (bbNext) edge + // and the OAE_NEXT_EDGE bit needs to be masked to obtain the real assertion index. + // Currently this is used by OAK_NO_THROW assertions but it may also be useful for other kinds of assertions + // by removing the need to create unnecessary complementary assertions. However, this bit twiddling mechanism + // is fragile and should be replaced with something cleaner (e.g. struct + bitfield). enum optAssertionEdge : AssertionIndex { // OAE_JUMP_EDGE = 0x0000, // assertion holds for bbJumpDest (default) diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h index cd1034d..0ef02d1 100644 --- a/src/coreclr/src/jit/gentree.h +++ b/src/coreclr/src/jit/gentree.h @@ -397,6 +397,7 @@ struct GenTree #if ASSERTION_PROP unsigned short gtAssertionNum; // 0 or Assertion table index + // possibly ORed with optAssertionEdge::OAE_NEXT_EDGE // valid only for non-GT_STMT nodes bool HasAssertion() const diff --git a/src/coreclr/src/jit/valuenum.cpp b/src/coreclr/src/jit/valuenum.cpp index 03cfb5b..b771bf6 100644 --- a/src/coreclr/src/jit/valuenum.cpp +++ b/src/coreclr/src/jit/valuenum.cpp @@ -3164,15 +3164,17 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) } //------------------------------------------------------------------------ -// IsVNArrLenUnsignedBound: Checks if the specified vn represents an -// expression such as "(uint)i < (uint)a.len" that imply that the -// array index is valid (0 <= i && i < a.len). +// IsVNArrLenUnsignedBound: Checks if the specified vn represents an expression +// such as "(uint)i < (uint)a.len" that implies that the array index is valid +// (0 <= i && i < a.len). // // Arguments: // vn - Value number to query -// info - Pointer to a ArrLenUnsignedBoundInfo object to return -// information about the expression. Not populated if the -// vn expression isn't suitable (e.g. i <= a.len) +// info - Pointer to an ArrLenUnsignedBoundInfo object to return information about +// the expression. Not populated if the vn expression isn't suitable (e.g. i <= a.len). +// This enables optCreateJTrueBoundAssertion to immediatly create an OAK_NO_THROW +// assertion instead of the OAK_EQUAL/NOT_EQUAL assertions created by signed compares +// (IsVNArrLenBound, IsVNArrLenArithBound) that require further processing. bool ValueNumStore::IsVNArrLenUnsignedBound(ValueNum vn, ArrLenUnsignedBoundInfo* info) { -- 2.7.4