[llvm] ff1147c - Revert "Ensure that InstructionCost actually implements a total ordering"

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 3 10:57:47 PST 2021


Please include information about why a patch is being reverted (links
to and quotes from buildbot failures, for instance) in the commit
message - helps when going back over patch history, or if someone's
seeing a break they can see if this patch will address their issue,
etc.

On Tue, Feb 2, 2021 at 12:12 PM Christopher Tetreault via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Christopher Tetreault
> Date: 2021-02-02T12:10:02-08:00
> New Revision: ff1147c3635685ba6aefbdc9394300adb5404595
>
> URL: https://github.com/llvm/llvm-project/commit/ff1147c3635685ba6aefbdc9394300adb5404595
> DIFF: https://github.com/llvm/llvm-project/commit/ff1147c3635685ba6aefbdc9394300adb5404595.diff
>
> LOG: Revert "Ensure that InstructionCost actually implements a total ordering"
>
> This reverts commit b481cd519e07b3ad2bd3e81c89b0dd8efd68d6bc.
>
> Added:
>
>
> Modified:
>     llvm/include/llvm/Support/InstructionCost.h
>     llvm/unittests/Support/InstructionCostTest.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff  --git a/llvm/include/llvm/Support/InstructionCost.h b/llvm/include/llvm/Support/InstructionCost.h
> index 7101ed1c9365..fbc898b878bb 100644
> --- a/llvm/include/llvm/Support/InstructionCost.h
> +++ b/llvm/include/llvm/Support/InstructionCost.h
> @@ -146,30 +146,31 @@ class InstructionCost {
>      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 to the comparison operators that
> -  /// the states are valid and users can test for validity of the cost
> -  /// explicitly.
> -  bool operator<(const InstructionCost &RHS) const {
> -    return State < RHS.State || Value < RHS.Value;
> -  }
> -
> -  // Implement in terms of operator< to ensure that the two comparisons stay in
> -  // sync
>    bool operator==(const InstructionCost &RHS) const {
> -    return !(*this < RHS) && !(RHS < *this);
> +    return State == RHS.State && Value == RHS.Value;
>    }
>
>    bool operator!=(const InstructionCost &RHS) const { return !(*this == RHS); }
>
>    bool operator==(const CostType RHS) const {
> -    InstructionCost RHS2(RHS);
> -    return *this == RHS2;
> +    return State == Valid && Value == RHS;
>    }
>
>    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); }
>
> diff  --git a/llvm/unittests/Support/InstructionCostTest.cpp b/llvm/unittests/Support/InstructionCostTest.cpp
> index 2a881a71e2e4..8ba9f990f027 100644
> --- a/llvm/unittests/Support/InstructionCostTest.cpp
> +++ b/llvm/unittests/Support/InstructionCostTest.cpp
> @@ -25,7 +25,6 @@ TEST_F(CostTest, Operators) {
>    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,9 +37,6 @@ TEST_F(CostTest, Operators) {
>    EXPECT_EQ(VThree - VNegTwo, 5);
>    EXPECT_EQ(VThree * VNegTwo, -6);
>    EXPECT_EQ(VSix / VThree, 2);
> -  EXPECT_NE(IThreeA, ITwo);
> -  EXPECT_LT(ITwo, IThreeA);
> -  EXPECT_GT(IThreeA, ITwo);
>
>    EXPECT_FALSE(IThreeA.isValid());
>    EXPECT_EQ(IThreeA.getState(), InstructionCost::Invalid);
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list