From f747a10753c23a24f2a7c6ba087b56a72b3f1972 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 30 Aug 2022 18:52:30 -0700 Subject: [PATCH 1/4] IsValidCompareChain should return 'false' if both operands are not integral types --- src/coreclr/jit/lowerarmarch.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 0ff88151503201..4e844eadc4aa7e 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2086,7 +2086,8 @@ bool Lowering::IsValidCompareChain(GenTree* child, GenTree* parent) return IsValidCompareChain(child->AsOp()->gtGetOp2(), child) && IsValidCompareChain(child->AsOp()->gtGetOp1(), child); } - else if (child->OperIsCmpCompare()) + else if (child->OperIsCmpCompare() && varTypeIsIntegral(child->gtGetOp1()) && + varTypeIsIntegral(child->gtGetOp2())) { // Can the child compare be contained. return IsSafeToContainMem(parent, child); From 42d844ed54dd379215e7e8b1830ebc410b9a78ef Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 31 Aug 2022 09:52:41 -0700 Subject: [PATCH 2/4] Add integral type checks in ContainCheckCompareChain --- src/coreclr/jit/lowerarmarch.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 4e844eadc4aa7e..f3abbc1078d07c 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2122,7 +2122,8 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree return true; } // Can the child be contained. - else if (IsSafeToContainMem(parent, child)) + else if (IsSafeToContainMem(parent, child) && varTypeIsIntegral(child->gtGetOp1()) && + varTypeIsIntegral(child->gtGetOp2())) { if (child->OperIs(GT_AND)) { From 9eaa8a1dfd1e69de2c6479fa10886ed318fbe677 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 31 Aug 2022 09:59:48 -0700 Subject: [PATCH 3/4] Check if op is CMP or AND --- src/coreclr/jit/lowerarmarch.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index f3abbc1078d07c..117bef334a6fb0 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2122,8 +2122,8 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree return true; } // Can the child be contained. - else if (IsSafeToContainMem(parent, child) && varTypeIsIntegral(child->gtGetOp1()) && - varTypeIsIntegral(child->gtGetOp2())) + else if (IsSafeToContainMem(parent, child) && (child->OperIs(GT_AND) || child->OperIsCmpCompare()) && + varTypeIsIntegral(child->gtGetOp1()) && varTypeIsIntegral(child->gtGetOp2())) { if (child->OperIs(GT_AND)) { From a4aa7cb65151dd5831866075789201c9385a6733 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Wed, 31 Aug 2022 14:35:31 -0700 Subject: [PATCH 4/4] feedback --- src/coreclr/jit/lowerarmarch.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 117bef334a6fb0..3cc7000f442595 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2122,8 +2122,7 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree return true; } // Can the child be contained. - else if (IsSafeToContainMem(parent, child) && (child->OperIs(GT_AND) || child->OperIsCmpCompare()) && - varTypeIsIntegral(child->gtGetOp1()) && varTypeIsIntegral(child->gtGetOp2())) + else if (IsSafeToContainMem(parent, child)) { if (child->OperIs(GT_AND)) { @@ -2150,7 +2149,8 @@ bool Lowering::ContainCheckCompareChain(GenTree* child, GenTree* parent, GenTree child->SetContained(); return true; } - else if (child->OperIsCmpCompare()) + else if (child->OperIsCmpCompare() && varTypeIsIntegral(child->gtGetOp1()) && + varTypeIsIntegral(child->gtGetOp2())) { child->AsOp()->SetContained();