[RS4GC] Cleanup meetBDVState. NFC.

[skatkov]

Summary

meetBDVState looks pretty difficult to read and follow.
This is purely NFC but doing several things:

1. Combine meet and meetBDVState
2. move the function under member of BDVState
3. Make BDVState be a mutable object.
4. Convert switch to sequence of ifs
5. Adds comments.

The patch is published to get a feedback whether this change simplifies the reading of the method.
If it is not, I'm fine to abandon it.

[reames]

LGTM

This basically reverts a couple of my recent changes, but I think I agree that the seperation I was trying for didn't work out well. Thanks for doing this.

llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

public:
   StatusTy getStatus() const { return Status; }
   Value *getOriginalValue() const { return OriginalValue; }
   Value *getBaseValue() const { return BaseValue; }
 
   bool isBase() const { return getStatus() == Base; }
   bool isUnknown() const { return getStatus() == Unknown; }
   bool isConflict() const { return getStatus() == Conflict; }
 
+  // Values of type BDVState form a lattice, and this function implements the
+  // meet
+  // operation.
+  void meet(const BDVState &Other) {
+    auto markConflict = [&]() {
+      Status = BDVState::Conflict;
+      BaseValue = nullptr;
+    };
+    // Conflict is a final state.
+    if (isConflict())
+      return;
+    // if we are not known - just take other state.
+    if (isUnknown()) {
+      Status = Other.getStatus();
+      BaseValue = Other.getBaseValue();
+      return;
+    }
+    // We are base.
+    assert(isBase() && "Unknown state");
+    // If other is unknown - just keep our state.
+    if (Other.isUnknown())
+      return;
+    // If other is conflict - it is a final state.
+    if (Other.isConflict())
+      return markConflict();
+    // Other is base as well.
+    assert(Other.isBase() && "Unknown state");
+    // If bases are different - Conflict.
+    if (getBaseValue() != Other.getBaseValue())
+      return markConflict();
+    // We are identical, do nothing.
+  }
+
   bool operator==(const BDVState &Other) const {
     return OriginalValue == OriginalValue && BaseValue == Other.BaseValue &&
       Status == Other.Status;
   }
 
   bool operator!=(const BDVState &other) const { return !(*this == other); }
 
   LLVM_DUMP_METHOD

 
 #ifndef NDEBUG
 static raw_ostream &operator<<(raw_ostream &OS, const BDVState &State) {
   State.print(OS);
   return OS;
 }
 #endif
 
-static BDVState::StatusTy meet(const BDVState::StatusTy &LHS,
-                               const BDVState::StatusTy &RHS) {
-  switch (LHS) {
-  case BDVState::Unknown:
-    return RHS;
-  case BDVState::Base:
-    switch (RHS) {
-    case BDVState::Unknown:
-    case BDVState::Base:
-      return BDVState::Base;
-    case BDVState::Conflict:
-      return BDVState::Conflict;
-    };
-    llvm_unreachable("covered switch");
-  case BDVState::Conflict:
-    return BDVState::Conflict;
-  }
-  llvm_unreachable("covered switch");
-}
-
-// Values of type BDVState form a lattice, and this function implements the meet
-// operation.
-static BDVState meetBDVState(const BDVState &LHS, const BDVState &RHS) {
-  auto NewStatus = meet(LHS.getStatus(), RHS.getStatus());
-  assert(NewStatus == meet(RHS.getStatus(), LHS.getStatus()));
-
-  Value *BaseValue = LHS.getStatus() == BDVState::Base ?
-    LHS.getBaseValue() : RHS.getBaseValue();
-  if (LHS.getStatus() == BDVState::Base && RHS.getStatus() == BDVState::Base &&
-      LHS.getBaseValue() != RHS.getBaseValue()) {
-    NewStatus = BDVState::Conflict;
-  }
-  if (NewStatus == BDVState::Conflict)
-    BaseValue = nullptr;
-  return BDVState(LHS.getOriginalValue(), NewStatus, BaseValue);
-}
-
 /// For a given value or instruction, figure out what base ptr its derived from.
 /// For gc objects, this is simply itself.  On success, returns a value which is
 /// the base pointer.  (This is reliable and can be used for relocation.)  On
 /// failure, returns nullptr.
 static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache) {
   Value *Def = findBaseOrBDV(I, Cache);
 
   if (isKnownBaseResult(Def) && areBothVectorOrScalar(Def, I))
for (auto Pair : States) {
       // Only values that do not have known bases or those that have differing
       // type (scalar versus vector) from a possible known base should be in the
       // lattice.
       assert((!isKnownBaseResult(BDV) ||
              !areBothVectorOrScalar(BDV, Pair.second.getBaseValue())) &&
                  "why did it get added?");
 
       BDVState NewState(BDV);
-      visitBDVOperands(BDV, [&] (Value *Op) {
+      visitBDVOperands(BDV, [&](Value *Op) {
         Value *BDV = findBaseOrBDV(Op, Cache);
         auto OpState = GetStateForBDV(BDV, Op);
-        NewState = meetBDVState(NewState, OpState);
+        NewState.meet(OpState);
       });
 
       BDVState OldState = States[BDV];
       if (OldState != NewState) {
         Progress = true;
         States[BDV] = NewState;
       }
     }