[PATCH] D95803: Ensure that InstructionCost actually implements a total ordering
Christopher Tetreault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 1 10:52:08 PST 2021
ctetreau created this revision.
Herald added a subscriber: dexonsmith.
ctetreau requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Previously, operator== would consider the actual equality of the pairs
(lhs.Value, lhs.State) == (rhs.Value, rhs.State). However, if an invalid
cost was involved in a call to operator<, only the state would be
compared. Thus, it was not the case that ({2, Invalid} < {3, Invalid} ||
{2, Invalid} > {3, Invalid} || {2, Invalid} == {3, Invalid}).
This patch implements a true total ordering, where cost state is
considered first, then value. While it's not really imporant that
{2, Invalid} be considered to be less than {3, Invalid}, it's not a
problem either. This patch also implements operator== in terms of
operator<, so the two definitions will be kept in sync.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D95803
Files:
llvm/include/llvm/Support/InstructionCost.h
llvm/unittests/Support/InstructionCostTest.cpp
Index: llvm/unittests/Support/InstructionCostTest.cpp
===================================================================
--- llvm/unittests/Support/InstructionCostTest.cpp
+++ llvm/unittests/Support/InstructionCostTest.cpp
@@ -25,6 +25,7 @@
InstructionCost VSix = 6;
InstructionCost IThreeA = InstructionCost::getInvalid(3);
InstructionCost IThreeB = InstructionCost::getInvalid(3);
+ InstructionCost ITwo = InstructionCost::getInvalid(2);
InstructionCost TmpCost;
EXPECT_NE(VThree, VNegTwo);
@@ -38,6 +39,11 @@
EXPECT_EQ(VThree * VNegTwo, -6);
EXPECT_EQ(VSix / VThree, 2);
+ EXPECT_TRUE((IThreeA < ITwo) || (IThreeA > ITwo) || (IThreeA == ITwo));
+ EXPECT_NE(IThreeA, ITwo);
+ EXPECT_LT(ITwo, IThreeA);
+ EXPECT_GT(IThreeA, ITwo);
+
EXPECT_FALSE(IThreeA.isValid());
EXPECT_EQ(IThreeA.getState(), InstructionCost::Invalid);
Index: llvm/include/llvm/Support/InstructionCost.h
===================================================================
--- llvm/include/llvm/Support/InstructionCost.h
+++ llvm/include/llvm/Support/InstructionCost.h
@@ -146,31 +146,30 @@
return Copy;
}
+ /// For the comparison operators we have chosen to use lexicographical
+ /// ordering where valid costs are always considered to be less than invalid
+ /// costs. This avoids having to add asserts the comparison operators that the
+ /// states are valid and users can test for validity of the cost explicitly.
+ bool operator<(const InstructionCost &RHS) const {
+ if (State < RHS.State)
+ return true;
+
+ return Value < RHS.Value;
+ }
+
bool operator==(const InstructionCost &RHS) const {
- return State == RHS.State && Value == RHS.Value;
+ return !(*this < RHS) && !(RHS < *this);
}
bool operator!=(const InstructionCost &RHS) const { return !(*this == RHS); }
bool operator==(const CostType RHS) const {
- return State == Valid && Value == RHS;
+ InstructionCost RHS2(RHS);
+ return *this == RHS2;
}
bool operator!=(const CostType RHS) const { return !(*this == RHS); }
- /// For the comparison operators we have chosen to use total ordering with
- /// the following rules:
- /// 1. If either of the states != Valid then a lexicographical order is
- /// applied based upon the state.
- /// 2. If both states are valid then order based upon value.
- /// This avoids having to add asserts the comparison operators that the states
- /// are valid and users can test for validity of the cost explicitly.
- bool operator<(const InstructionCost &RHS) const {
- if (State != Valid || RHS.State != Valid)
- return State < RHS.State;
- return Value < RHS.Value;
- }
-
bool operator>(const InstructionCost &RHS) const { return RHS < *this; }
bool operator<=(const InstructionCost &RHS) const { return !(RHS < *this); }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D95803.320525.patch
Type: text/x-patch
Size: 2833 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210201/6e5c43eb/attachment.bin>
More information about the llvm-commits
mailing list