Skip to content

Commit 7ce5d0f

Browse files
authored
JIT: Expand optRelopImpliesRelop for constants as op1 (#102024)
VN does not guarantee that constants in relops are first, so swap them if this isn't the case. This allows the logic to prove the value of more relops.
1 parent bdfbed6 commit 7ce5d0f

File tree

2 files changed

+116
-59
lines changed

2 files changed

+116
-59
lines changed

src/coreclr/jit/compiler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7502,6 +7502,9 @@ class Compiler
75027502
BitVecTraits* optReachableBitVecTraits;
75037503
BitVec optReachableBitVec;
75047504
void optRelopImpliesRelop(RelopImplicationInfo* rii);
7505+
bool optRelopTryInferWithOneEqualOperand(const VNFuncApp& domApp,
7506+
const VNFuncApp& treeApp,
7507+
RelopImplicationInfo* rii);
75057508

75067509
/**************************************************************************
75077510
* Value/Assertion propagation

src/coreclr/jit/redundantbranchopts.cpp

Lines changed: 113 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -509,66 +509,11 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii)
509509
}
510510
}
511511

512-
// Given R(x, cns1) and R*(x, cns2) see if we can infer R* from R.
513-
// We assume cns1 and cns2 are always on the RHS of the compare.
514-
if ((treeApp.m_args[0] == domApp.m_args[0]) && vnStore->IsVNConstant(treeApp.m_args[1]) &&
515-
vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) &&
516-
vnStore->TypeOfVN(domApp.m_args[0]) == vnStore->TypeOfVN(treeApp.m_args[1]) &&
517-
vnStore->TypeOfVN(domApp.m_args[1]) == vnStore->TypeOfVN(treeApp.m_args[1]))
512+
if (((treeApp.m_args[0] == domApp.m_args[0]) || (treeApp.m_args[0] == domApp.m_args[1]) ||
513+
(treeApp.m_args[1] == domApp.m_args[0]) || (treeApp.m_args[1] == domApp.m_args[1])) &&
514+
optRelopTryInferWithOneEqualOperand(domApp, treeApp, rii))
518515
{
519-
// We currently don't handle VNF_relop_UN funcs here
520-
if (ValueNumStore::VNFuncIsSignedComparison(domApp.m_func) &&
521-
ValueNumStore::VNFuncIsSignedComparison(treeApp.m_func))
522-
{
523-
// Dominating "X relop CNS"
524-
const genTreeOps domOper = static_cast<genTreeOps>(domApp.m_func);
525-
const target_ssize_t domCns = vnStore->CoercedConstantValue<target_ssize_t>(domApp.m_args[1]);
526-
527-
// Dominated "X relop CNS"
528-
const genTreeOps treeOper = static_cast<genTreeOps>(treeApp.m_func);
529-
const target_ssize_t treeCns = vnStore->CoercedConstantValue<target_ssize_t>(treeApp.m_args[1]);
530-
531-
// Example:
532-
//
533-
// void Test(int x)
534-
// {
535-
// if (x > 100)
536-
// if (x > 10)
537-
// Console.WriteLine("Taken!");
538-
// }
539-
//
540-
541-
// Corresponding BB layout:
542-
//
543-
// BB1:
544-
// if (x <= 100)
545-
// goto BB4
546-
//
547-
// BB2:
548-
// // x is known to be > 100 here
549-
// if (x <= 10) // never true
550-
// goto BB4
551-
//
552-
// BB3:
553-
// Console.WriteLine("Taken!");
554-
//
555-
// BB4:
556-
// return;
557-
558-
// Check whether the dominating compare being "false" implies the dominated compare is known
559-
// to be either "true" or "false".
560-
RelopResult treeOperStatus =
561-
IsCmp2ImpliedByCmp1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns);
562-
if (treeOperStatus != RelopResult::Unknown)
563-
{
564-
rii->canInfer = true;
565-
rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred;
566-
rii->canInferFromTrue = false;
567-
rii->canInferFromFalse = true;
568-
rii->reverseSense = treeOperStatus == RelopResult::AlwaysTrue;
569-
return;
570-
}
571-
}
516+
return;
572517
}
573518
}
574519

@@ -646,6 +591,115 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii)
646591
}
647592
}
648593

594+
//------------------------------------------------------------------------
595+
// optRelopTryInferWithOneEqualOperand: Given a domnating relop R(x, y) and
596+
// another relop R*(a, b) that share an operand, try to see if we can infer
597+
// something about R*(a, b).
598+
//
599+
// Arguments:
600+
// domApp - The dominating relop R*(x, y)
601+
// treeApp - The dominated relop R*(a, b)
602+
// rii - [out] struct with relop implication information
603+
//
604+
// Returns:
605+
// True if something was inferred; otherwise false.
606+
//
607+
bool Compiler::optRelopTryInferWithOneEqualOperand(const VNFuncApp& domApp,
608+
const VNFuncApp& treeApp,
609+
RelopImplicationInfo* rii)
610+
{
611+
// Canonicalize constants to be on the right.
612+
VNFunc domFunc = domApp.m_func;
613+
ValueNum domOp1 = domApp.m_args[0];
614+
ValueNum domOp2 = domApp.m_args[1];
615+
616+
VNFunc treeFunc = treeApp.m_func;
617+
ValueNum treeOp1 = treeApp.m_args[0];
618+
ValueNum treeOp2 = treeApp.m_args[1];
619+
620+
if (vnStore->IsVNConstant(domOp1))
621+
{
622+
std::swap(domOp1, domOp2);
623+
domFunc = ValueNumStore::SwapRelop(domFunc);
624+
}
625+
626+
if (vnStore->IsVNConstant(treeOp1))
627+
{
628+
std::swap(treeOp1, treeOp2);
629+
treeFunc = ValueNumStore::SwapRelop(treeFunc);
630+
}
631+
632+
// Given R(x, cns1) and R*(x, cns2) see if we can infer R* from R.
633+
if ((treeOp1 != domOp1) || !vnStore->IsVNConstant(treeOp2) || !vnStore->IsVNConstant(domOp2))
634+
{
635+
return false;
636+
}
637+
638+
var_types treeOp1Type = vnStore->TypeOfVN(treeOp1);
639+
var_types treeOp2Type = vnStore->TypeOfVN(treeOp2);
640+
var_types domOp1Type = vnStore->TypeOfVN(domOp1);
641+
var_types domOp2Type = vnStore->TypeOfVN(domOp2);
642+
if (!varTypeIsIntOrI(treeOp1Type) || (domOp1Type != treeOp2Type) || (domOp2Type != treeOp2Type))
643+
{
644+
return false;
645+
}
646+
// We currently don't handle VNF_relop_UN funcs here
647+
if (!ValueNumStore::VNFuncIsSignedComparison(domFunc) || !ValueNumStore::VNFuncIsSignedComparison(treeFunc))
648+
{
649+
return false;
650+
}
651+
652+
// Dominating "X relop CNS"
653+
const genTreeOps domOper = static_cast<genTreeOps>(domFunc);
654+
const target_ssize_t domCns = vnStore->CoercedConstantValue<target_ssize_t>(domOp2);
655+
656+
// Dominated "X relop CNS"
657+
const genTreeOps treeOper = static_cast<genTreeOps>(treeFunc);
658+
const target_ssize_t treeCns = vnStore->CoercedConstantValue<target_ssize_t>(treeOp2);
659+
660+
// Example:
661+
//
662+
// void Test(int x)
663+
// {
664+
// if (x > 100)
665+
// if (x > 10)
666+
// Console.WriteLine("Taken!");
667+
// }
668+
//
669+
670+
// Corresponding BB layout:
671+
//
672+
// BB1:
673+
// if (x <= 100)
674+
// goto BB4
675+
//
676+
// BB2:
677+
// // x is known to be > 100 here
678+
// if (x <= 10) // never true
679+
// goto BB4
680+
//
681+
// BB3:
682+
// Console.WriteLine("Taken!");
683+
//
684+
// BB4:
685+
// return;
686+
687+
// Check whether the dominating compare being "false" implies the dominated compare is known
688+
// to be either "true" or "false".
689+
RelopResult treeOperStatus = IsCmp2ImpliedByCmp1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns);
690+
if (treeOperStatus == RelopResult::Unknown)
691+
{
692+
return false;
693+
}
694+
695+
rii->canInfer = true;
696+
rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred;
697+
rii->canInferFromTrue = false;
698+
rii->canInferFromFalse = true;
699+
rii->reverseSense = treeOperStatus == RelopResult::AlwaysTrue;
700+
return true;
701+
}
702+
649703
//------------------------------------------------------------------------
650704
// optRedundantBranch: try and optimize a possibly redundant branch
651705
//

0 commit comments

Comments
 (0)