Revert "[IR] Don't store switch case values as operands" (#170962)

Reverts #166842

Breaks Mips LLVM tests, and LLD on bots.
See #166842

Author: vitalybuka

llvm/docs/ReleaseNotes.md

@@ -74,7 +74,6 @@ Changes to the LLVM IR
   format string function implementations from statically-linked libc's based on
   the requirements of each call. Currently only `float` is supported; this can
   keep floating point support out of printf if it can be proven unused.
-* Case values are no longer operands of `SwitchInst`.
 
 Changes to LLVM infrastructure
 ------------------------------
@@ -179,7 +178,6 @@ Changes to the C API
 * Add `LLVMGetOrInsertFunction` to get or insert a function, replacing the combination of `LLVMGetNamedFunction` and `LLVMAddFunction`.
 * Allow `LLVMGetVolatile` to work with any kind of Instruction.
 * Add `LLVMConstFPFromBits` to get a constant floating-point value from an array of 64 bit values.
-* Add `LLVMGetSwitchCaseValue` and `LLVMSetSwitchCaseValue` to get and set switch case values; switch case values are no longer operands of the instruction.
 
 Changes to the CodeGen infrastructure
 -------------------------------------

llvm/include/llvm-c/Core.h

@@ -4213,30 +4213,6 @@ LLVM_C_ABI void LLVMSetCondition(LLVMValueRef Branch, LLVMValueRef Cond);
  */
 LLVM_C_ABI LLVMBasicBlockRef LLVMGetSwitchDefaultDest(LLVMValueRef SwitchInstr);
 
-/**
- * Obtain the case value for a successor of a switch instruction. i corresponds
- * to the successor index. The first successor is the default destination, so i
- * must be greater than zero.
- *
- * This only works on llvm::SwitchInst instructions.
- *
- * @see llvm::SwitchInst::CaseHandle::getCaseValue()
- */
-LLVM_C_ABI LLVMValueRef LLVMGetSwitchCaseValue(LLVMValueRef SwitchInstr,
-                                               unsigned i);
-
-/**
- * Set the case value for a successor of a switch instruction. i corresponds to
- * the successor index. The first successor is the default destination, so i
- * must be greater than zero.
- *
- * This only works on llvm::SwitchInst instructions.
- *
- * @see llvm::SwitchInst::CaseHandle::setValue()
- */
-LLVM_C_ABI void LLVMSetSwitchCaseValue(LLVMValueRef SwitchInstr, unsigned i,
-                                       LLVMValueRef CaseValue);
-
 /**
  * @}
  */

llvm/include/llvm/IR/Instructions.h

@@ -2665,7 +2665,7 @@ class PHINode : public Instruction {
   // User::allocHungoffUses, because we have to allocate Uses for the incoming
   // values and pointers to the incoming blocks, all in one allocation.
   void allocHungoffUses(unsigned N) {
-    User::allocHungoffUses(N, /*WithExtraValues=*/true);
+    User::allocHungoffUses(N, /* IsPhi */ true);
   }
 
 public:
@@ -3198,10 +3198,10 @@ class SwitchInst : public Instruction {
 
   unsigned ReservedSpace;
 
-  // Operand[0] = Value to switch on
-  // Operand[1] = Default basic block destination
-  // Operand[n] = BasicBlock to go to on match
-  // Values are stored after the Uses similar to PHINode's basic blocks.
+  // Operand[0]    = Value to switch on
+  // Operand[1]    = Default basic block destination
+  // Operand[2n  ] = Value to match
+  // Operand[2n+1] = BasicBlock to go to on match
   SwitchInst(const SwitchInst &SI);
 
   /// Create a new switch instruction, specifying a value to switch on and a
@@ -3223,17 +3223,6 @@ class SwitchInst : public Instruction {
 
   LLVM_ABI SwitchInst *cloneImpl() const;
 
-  void allocHungoffUses(unsigned N) {
-    User::allocHungoffUses(N, /*WithExtraValues=*/true);
-  }
-
-  ConstantInt *const *case_values() const {
-    return reinterpret_cast<ConstantInt *const *>(op_begin() + ReservedSpace);
-  }
-  ConstantInt **case_values() {
-    return reinterpret_cast<ConstantInt **>(op_begin() + ReservedSpace);
-  }
-
 public:
   void operator delete(void *Ptr) { User::operator delete(Ptr); }
 
@@ -3268,7 +3257,7 @@ class SwitchInst : public Instruction {
     ConstantIntT *getCaseValue() const {
       assert((unsigned)Index < SI->getNumCases() &&
              "Index out the number of cases.");
-      return SI->case_values()[Index];
+      return reinterpret_cast<ConstantIntT *>(SI->getOperand(2 + Index * 2));
     }
 
     /// Resolves successor for current case.
@@ -3310,7 +3299,7 @@ class SwitchInst : public Instruction {
     void setValue(ConstantInt *V) const {
       assert((unsigned)Index < SI->getNumCases() &&
              "Index out the number of cases.");
-      SI->case_values()[Index] = V;
+      SI->setOperand(2 + Index*2, reinterpret_cast<Value*>(V));
     }
 
     /// Sets the new successor for current case.
@@ -3417,7 +3406,9 @@ class SwitchInst : public Instruction {
 
   /// Return the number of 'cases' in this switch instruction, excluding the
   /// default case.
-  unsigned getNumCases() const { return getNumOperands() - 2; }
+  unsigned getNumCases() const {
+    return getNumOperands()/2 - 1;
+  }
 
   /// Returns a read/write iterator that points to the first case in the
   /// SwitchInst.
@@ -3519,14 +3510,14 @@ class SwitchInst : public Instruction {
   /// case.
   LLVM_ABI CaseIt removeCase(CaseIt I);
 
-  unsigned getNumSuccessors() const { return getNumOperands() - 1; }
+  unsigned getNumSuccessors() const { return getNumOperands()/2; }
   BasicBlock *getSuccessor(unsigned idx) const {
     assert(idx < getNumSuccessors() &&"Successor idx out of range for switch!");
-    return cast<BasicBlock>(getOperand(idx + 1));
+    return cast<BasicBlock>(getOperand(idx*2+1));
   }
   void setSuccessor(unsigned idx, BasicBlock *NewSucc) {
     assert(idx < getNumSuccessors() && "Successor # out of range for switch!");
-    setOperand(idx + 1, NewSucc);
+    setOperand(idx * 2 + 1, NewSucc);
   }
 
   // Methods for support type inquiry through isa, cast, and dyn_cast:

llvm/include/llvm/IR/User.h

@@ -132,13 +132,13 @@ class User : public Value {
 
   /// Allocate the array of Uses, followed by a pointer
   /// (with bottom bit set) to the User.
-  /// \param WithExtraValues identifies callers which need N Value* allocated
-  /// along the N operands.
-  LLVM_ABI void allocHungoffUses(unsigned N, bool WithExtraValues = false);
+  /// \param IsPhi identifies callers which are phi nodes and which need
+  /// N BasicBlock* allocated along with N
+  LLVM_ABI void allocHungoffUses(unsigned N, bool IsPhi = false);
 
   /// Grow the number of hung off uses.  Note that allocHungoffUses
   /// should be called if there are no uses.
-  LLVM_ABI void growHungoffUses(unsigned N, bool WithExtraValues = false);
+  LLVM_ABI void growHungoffUses(unsigned N, bool IsPhi = false);
 
 protected:
   ~User() = default; // Use deleteValue() to delete a generic Instruction.

llvm/lib/Bitcode/Writer/ValueEnumerator.cpp

@@ -164,10 +164,6 @@ static OrderMap orderModule(const Module &M) {
           orderConstantValue(Op);
         if (auto *SVI = dyn_cast<ShuffleVectorInst>(&I))
           orderValue(SVI->getShuffleMaskForBitcode(), OM);
-        if (auto *SI = dyn_cast<SwitchInst>(&I)) {
-          for (const auto &Case : SI->cases())
-            orderValue(Case.getCaseValue(), OM);
-        }
         orderValue(&I, OM);
       }
   }
@@ -1096,10 +1092,6 @@ void ValueEnumerator::incorporateFunction(const Function &F) {
       }
       if (auto *SVI = dyn_cast<ShuffleVectorInst>(&I))
         EnumerateValue(SVI->getShuffleMaskForBitcode());
-      if (auto *SI = dyn_cast<SwitchInst>(&I)) {
-        for (const auto &Case : SI->cases())
-          EnumerateValue(Case.getCaseValue());
-      }
     }
     BasicBlocks.push_back(&BB);
     ValueMap[&BB] = BasicBlocks.size();

llvm/lib/CodeGen/TypePromotion.cpp

@@ -512,14 +512,6 @@ void IRPromoter::PromoteTree() {
         I->setOperand(i, ConstantInt::get(ExtTy, 0));
     }
 
-    // For switch, also mutate case values, which are not operands.
-    if (auto *SI = dyn_cast<SwitchInst>(I)) {
-      for (auto Case : SI->cases()) {
-        APInt NewConst = Case.getCaseValue()->getValue().zext(PromotedWidth);
-        Case.setValue(ConstantInt::get(SI->getContext(), NewConst));
-      }
-    }
-
     // Mutate the result type, unless this is an icmp or switch.
     if (!isa<ICmpInst>(I) && !isa<SwitchInst>(I)) {
       I->mutateType(ExtTy);

llvm/lib/IR/Core.cpp

@@ -3257,19 +3257,6 @@ LLVMBasicBlockRef LLVMGetSwitchDefaultDest(LLVMValueRef Switch) {
   return wrap(unwrap<SwitchInst>(Switch)->getDefaultDest());
 }
 
-LLVMValueRef LLVMGetSwitchCaseValue(LLVMValueRef Switch, unsigned i) {
-  assert(i > 0 && i <= unwrap<SwitchInst>(Switch)->getNumCases());
-  auto It = unwrap<SwitchInst>(Switch)->case_begin() + (i - 1);
-  return wrap(It->getCaseValue());
-}
-
-void LLVMSetSwitchCaseValue(LLVMValueRef Switch, unsigned i,
-                            LLVMValueRef CaseValue) {
-  assert(i > 0 && i <= unwrap<SwitchInst>(Switch)->getNumCases());
-  auto It = unwrap<SwitchInst>(Switch)->case_begin() + (i - 1);
-  It->setValue(unwrap<ConstantInt>(CaseValue));
-}
-
 /*--.. Operations on alloca instructions (only) ............................--*/
 
 LLVMTypeRef LLVMGetAllocatedType(LLVMValueRef Alloca) {

llvm/lib/IR/Instructions.cpp

@@ -202,7 +202,7 @@ void PHINode::growOperands() {
   if (NumOps < 2) NumOps = 2;      // 2 op PHI nodes are VERY common.
 
   ReservedSpace = NumOps;
-  growHungoffUses(ReservedSpace, /*WithExtraValues=*/true);
+  growHungoffUses(ReservedSpace, /* IsPhi */ true);
 }
 
 /// hasConstantValue - If the specified PHI node always merges together the same
@@ -4076,20 +4076,18 @@ SwitchInst::SwitchInst(Value *Value, BasicBlock *Default, unsigned NumCases,
                        InsertPosition InsertBefore)
     : Instruction(Type::getVoidTy(Value->getContext()), Instruction::Switch,
                   AllocMarker, InsertBefore) {
-  init(Value, Default, 2 + NumCases);
+  init(Value, Default, 2+NumCases*2);
 }
 
 SwitchInst::SwitchInst(const SwitchInst &SI)
     : Instruction(SI.getType(), Instruction::Switch, AllocMarker) {
   init(SI.getCondition(), SI.getDefaultDest(), SI.getNumOperands());
   setNumHungOffUseOperands(SI.getNumOperands());
   Use *OL = getOperandList();
-  ConstantInt **VL = case_values();
   const Use *InOL = SI.getOperandList();
-  ConstantInt *const *InVL = SI.case_values();
-  for (unsigned i = 2, E = SI.getNumOperands(); i != E; ++i) {
+  for (unsigned i = 2, E = SI.getNumOperands(); i != E; i += 2) {
     OL[i] = InOL[i];
-    VL[i - 2] = InVL[i - 2];
+    OL[i+1] = InOL[i+1];
   }
   SubclassOptionalData = SI.SubclassOptionalData;
 }
@@ -4099,11 +4097,11 @@ SwitchInst::SwitchInst(const SwitchInst &SI)
 void SwitchInst::addCase(ConstantInt *OnVal, BasicBlock *Dest) {
   unsigned NewCaseIdx = getNumCases();
   unsigned OpNo = getNumOperands();
-  if (OpNo + 1 > ReservedSpace)
+  if (OpNo+2 > ReservedSpace)
     growOperands();  // Get more space!
   // Initialize some new operands.
-  assert(OpNo < ReservedSpace && "Growing didn't work!");
-  setNumHungOffUseOperands(OpNo + 1);
+  assert(OpNo+1 < ReservedSpace && "Growing didn't work!");
+  setNumHungOffUseOperands(OpNo+2);
   CaseHandle Case(this, NewCaseIdx);
   Case.setValue(OnVal);
   Case.setSuccessor(Dest);
@@ -4114,22 +4112,21 @@ void SwitchInst::addCase(ConstantInt *OnVal, BasicBlock *Dest) {
 SwitchInst::CaseIt SwitchInst::removeCase(CaseIt I) {
   unsigned idx = I->getCaseIndex();
 
-  assert(2 + idx < getNumOperands() && "Case index out of range!!!");
+  assert(2 + idx*2 < getNumOperands() && "Case index out of range!!!");
 
   unsigned NumOps = getNumOperands();
   Use *OL = getOperandList();
-  ConstantInt **VL = case_values();
 
   // Overwrite this case with the end of the list.
-  if (2 + idx + 1 != NumOps) {
-    OL[2 + idx] = OL[NumOps - 1];
-    VL[idx] = VL[NumOps - 2 - 1];
+  if (2 + (idx + 1) * 2 != NumOps) {
+    OL[2 + idx * 2] = OL[NumOps - 2];
+    OL[2 + idx * 2 + 1] = OL[NumOps - 1];
   }
 
   // Nuke the last value.
-  OL[NumOps - 1].set(nullptr);
-  VL[NumOps - 2 - 1] = nullptr;
-  setNumHungOffUseOperands(NumOps - 1);
+  OL[NumOps-2].set(nullptr);
+  OL[NumOps-2+1].set(nullptr);
+  setNumHungOffUseOperands(NumOps-2);
 
   return CaseIt(this, idx);
 }
@@ -4142,7 +4139,7 @@ void SwitchInst::growOperands() {
   unsigned NumOps = e*3;
 
   ReservedSpace = NumOps;
-  growHungoffUses(ReservedSpace, /*WithExtraValues=*/true);
+  growHungoffUses(ReservedSpace);
 }
 
 void SwitchInstProfUpdateWrapper::init() {

llvm/lib/IR/User.cpp

@@ -50,24 +50,24 @@ bool User::replaceUsesOfWith(Value *From, Value *To) {
 //                         User allocHungoffUses Implementation
 //===----------------------------------------------------------------------===//
 
-void User::allocHungoffUses(unsigned N, bool WithExtraValues) {
+void User::allocHungoffUses(unsigned N, bool IsPhi) {
   assert(HasHungOffUses && "alloc must have hung off uses");
 
-  static_assert(alignof(Use) >= alignof(Value *),
+  static_assert(alignof(Use) >= alignof(BasicBlock *),
                 "Alignment is insufficient for 'hung-off-uses' pieces");
 
   // Allocate the array of Uses
   size_t size = N * sizeof(Use);
-  if (WithExtraValues)
-    size += N * sizeof(Value *);
+  if (IsPhi)
+    size += N * sizeof(BasicBlock *);
   Use *Begin = static_cast<Use*>(::operator new(size));
   Use *End = Begin + N;
   setOperandList(Begin);
   for (; Begin != End; Begin++)
     new (Begin) Use(this);
 }
 
-void User::growHungoffUses(unsigned NewNumUses, bool WithExtraValues) {
+void User::growHungoffUses(unsigned NewNumUses, bool IsPhi) {
   assert(HasHungOffUses && "realloc must have hung off uses");
 
   unsigned OldNumUses = getNumOperands();
@@ -77,22 +77,22 @@ void User::growHungoffUses(unsigned NewNumUses, bool WithExtraValues) {
   assert(NewNumUses > OldNumUses && "realloc must grow num uses");
 
   Use *OldOps = getOperandList();
-  allocHungoffUses(NewNumUses, WithExtraValues);
+  allocHungoffUses(NewNumUses, IsPhi);
   Use *NewOps = getOperandList();
 
   // Now copy from the old operands list to the new one.
   std::copy(OldOps, OldOps + OldNumUses, NewOps);
 
-  // If the User has extra values (phi basic blocks, switch case values), then
-  // we need to copy these, too.
-  if (WithExtraValues) {
+  // If this is a Phi, then we need to copy the BB pointers too.
+  if (IsPhi) {
     auto *OldPtr = reinterpret_cast<char *>(OldOps + OldNumUses);
     auto *NewPtr = reinterpret_cast<char *>(NewOps + NewNumUses);
-    std::copy(OldPtr, OldPtr + (OldNumUses * sizeof(Value *)), NewPtr);
+    std::copy(OldPtr, OldPtr + (OldNumUses * sizeof(BasicBlock *)), NewPtr);
   }
   Use::zap(OldOps, OldOps + OldNumUses, true);
 }
 
+
 // This is a private struct used by `User` to track the co-allocated descriptor
 // section.
 struct DescriptorInfo {

llvm/lib/IR/Verifier.cpp

@@ -3424,7 +3424,7 @@ void Verifier::visitSwitchInst(SwitchInst &SI) {
   Type *SwitchTy = SI.getCondition()->getType();
   SmallPtrSet<ConstantInt*, 32> Constants;
   for (auto &Case : SI.cases()) {
-    Check(isa<ConstantInt>(Case.getCaseValue()),
+    Check(isa<ConstantInt>(SI.getOperand(Case.getCaseIndex() * 2 + 2)),
           "Case value is not a constant integer.", &SI);
     Check(Case.getCaseValue()->getType() == SwitchTy,
           "Switch constants must all be same type as switch value!", &SI);