[IR] Don't store switch case values as operands

SwitchInst case values must be ConstantInt, which have no use list.
Therefore it is not necessary to store these as Use, instead store them
more efficiently as a simple array of pointers after the uses, similar
to how PHINode stores basic blocks.

After this change, the successors of all terminators are stored
consecutively in the operand list. This is preparatory work for
improving the performance of successor access.

Add new C API functions so that switch case values remain accessible
from bindings for other languages.

While this could also be achieved by merely changing the order of
operands (i.e., first all successors, then all constants), doing so
would increase the asymptotic runtime of addCase from O(1) to O(n)
(i.e., adding n cases would be O(n^2)), because it would need to shift
all constants by one slot. Having null/invalid operands is also a bad
idea and would cause much more breakage.

Pull Request: #170984

Author: aengelke

llvm/docs/ReleaseNotes.md

@@ -74,6 +74,7 @@ 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
 ------------------------------
@@ -217,6 +218,7 @@ Changes to the C API
   * `LLVMParseBitcode2` -> `LLVMParseBitcodeInContext2`
   * `LLVMGetBitcodeModule` -> `LLVMGetBitcodeModuleInContext2`
   * `LLVMGetBitcodeModule2` -> `LLVMGetBitcodeModuleInContext2`
+* 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

@@ -4296,6 +4296,30 @@ 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, /* IsPhi */ true);
+    User::allocHungoffUses(N, /*WithExtraValues=*/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[2n  ] = Value to match
-  // Operand[2n+1] = BasicBlock to go to on match
+  // 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.
   SwitchInst(const SwitchInst &SI);
 
   /// Create a new switch instruction, specifying a value to switch on and a
@@ -3223,6 +3223,17 @@ 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); }
 
@@ -3257,7 +3268,7 @@ class SwitchInst : public Instruction {
     ConstantIntT *getCaseValue() const {
       assert((unsigned)Index < SI->getNumCases() &&
              "Index out the number of cases.");
-      return reinterpret_cast<ConstantIntT *>(SI->getOperand(2 + Index * 2));
+      return SI->case_values()[Index];
     }
 
     /// Resolves successor for current case.
@@ -3299,7 +3310,7 @@ class SwitchInst : public Instruction {
     void setValue(ConstantInt *V) const {
       assert((unsigned)Index < SI->getNumCases() &&
              "Index out the number of cases.");
-      SI->setOperand(2 + Index*2, reinterpret_cast<Value*>(V));
+      SI->case_values()[Index] = V;
     }
 
     /// Sets the new successor for current case.
@@ -3406,9 +3417,7 @@ class SwitchInst : public Instruction {
 
   /// Return the number of 'cases' in this switch instruction, excluding the
   /// default case.
-  unsigned getNumCases() const {
-    return getNumOperands()/2 - 1;
-  }
+  unsigned getNumCases() const { return getNumOperands() - 2; }
 
   /// Returns a read/write iterator that points to the first case in the
   /// SwitchInst.
@@ -3510,14 +3519,14 @@ class SwitchInst : public Instruction {
   /// case.
   LLVM_ABI CaseIt removeCase(CaseIt I);
 
-  unsigned getNumSuccessors() const { return getNumOperands()/2; }
+  unsigned getNumSuccessors() const { return getNumOperands() - 1; }
   BasicBlock *getSuccessor(unsigned idx) const {
     assert(idx < getNumSuccessors() &&"Successor idx out of range for switch!");
-    return cast<BasicBlock>(getOperand(idx*2+1));
+    return cast<BasicBlock>(getOperand(idx + 1));
   }
   void setSuccessor(unsigned idx, BasicBlock *NewSucc) {
     assert(idx < getNumSuccessors() && "Successor # out of range for switch!");
-    setOperand(idx * 2 + 1, NewSucc);
+    setOperand(idx + 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 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);
+  /// \param WithExtraValues identifies callers which need N Value* allocated
+  /// along the N operands.
+  LLVM_ABI void allocHungoffUses(unsigned N, bool WithExtraValues = 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 IsPhi = false);
+  LLVM_ABI void growHungoffUses(unsigned N, bool WithExtraValues = false);
 
 protected:
   ~User() = default; // Use deleteValue() to delete a generic Instruction.

llvm/lib/Bitcode/Writer/ValueEnumerator.cpp

@@ -164,6 +164,10 @@ 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);
       }
   }
@@ -1092,6 +1096,10 @@ 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,6 +512,14 @@ 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

@@ -3261,6 +3261,19 @@ 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/Instruction.cpp

@@ -865,7 +865,7 @@ const char *Instruction::getOpcodeName(unsigned OpCode) {
 }
 
 /// This must be kept in sync with FunctionComparator::cmpOperations in
-/// lib/Transforms/IPO/MergeFunctions.cpp.
+/// lib/Transforms/Utils/FunctionComparator.cpp.
 bool Instruction::hasSameSpecialState(const Instruction *I2,
                                       bool IgnoreAlignment,
                                       bool IntersectAttrs) const {
@@ -913,6 +913,12 @@ bool Instruction::hasSameSpecialState(const Instruction *I2,
     return CI->getCallingConv() == cast<CallBrInst>(I2)->getCallingConv() &&
            CheckAttrsSame(CI, cast<CallBrInst>(I2)) &&
            CI->hasIdenticalOperandBundleSchema(*cast<CallBrInst>(I2));
+  if (const SwitchInst *SI = dyn_cast<SwitchInst>(I1)) {
+    for (auto [Case1, Case2] : zip(SI->cases(), cast<SwitchInst>(I2)->cases()))
+      if (Case1.getCaseValue() != Case2.getCaseValue())
+        return false;
+    return true;
+  }
   if (const InsertValueInst *IVI = dyn_cast<InsertValueInst>(I1))
     return IVI->getIndices() == cast<InsertValueInst>(I2)->getIndices();
   if (const ExtractValueInst *EVI = dyn_cast<ExtractValueInst>(I1))

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, /* IsPhi */ true);
+  growHungoffUses(ReservedSpace, /*WithExtraValues=*/true);
 }
 
 /// hasConstantValue - If the specified PHI node always merges together the same
@@ -4076,18 +4076,20 @@ SwitchInst::SwitchInst(Value *Value, BasicBlock *Default, unsigned NumCases,
                        InsertPosition InsertBefore)
     : Instruction(Type::getVoidTy(Value->getContext()), Instruction::Switch,
                   AllocMarker, InsertBefore) {
-  init(Value, Default, 2+NumCases*2);
+  init(Value, Default, 2 + NumCases);
 }
 
 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();
-  for (unsigned i = 2, E = SI.getNumOperands(); i != E; i += 2) {
+  ConstantInt *const *InVL = SI.case_values();
+  for (unsigned i = 2, E = SI.getNumOperands(); i != E; ++i) {
     OL[i] = InOL[i];
-    OL[i+1] = InOL[i+1];
+    VL[i - 2] = InVL[i - 2];
   }
   SubclassOptionalData = SI.SubclassOptionalData;
 }
@@ -4097,11 +4099,11 @@ SwitchInst::SwitchInst(const SwitchInst &SI)
 void SwitchInst::addCase(ConstantInt *OnVal, BasicBlock *Dest) {
   unsigned NewCaseIdx = getNumCases();
   unsigned OpNo = getNumOperands();
-  if (OpNo+2 > ReservedSpace)
+  if (OpNo + 1 > ReservedSpace)
     growOperands();  // Get more space!
   // Initialize some new operands.
-  assert(OpNo+1 < ReservedSpace && "Growing didn't work!");
-  setNumHungOffUseOperands(OpNo+2);
+  assert(OpNo < ReservedSpace && "Growing didn't work!");
+  setNumHungOffUseOperands(OpNo + 1);
   CaseHandle Case(this, NewCaseIdx);
   Case.setValue(OnVal);
   Case.setSuccessor(Dest);
@@ -4112,21 +4114,22 @@ void SwitchInst::addCase(ConstantInt *OnVal, BasicBlock *Dest) {
 SwitchInst::CaseIt SwitchInst::removeCase(CaseIt I) {
   unsigned idx = I->getCaseIndex();
 
-  assert(2 + idx*2 < getNumOperands() && "Case index out of range!!!");
+  assert(2 + idx < 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) * 2 != NumOps) {
-    OL[2 + idx * 2] = OL[NumOps - 2];
-    OL[2 + idx * 2 + 1] = OL[NumOps - 1];
+  if (2 + idx + 1 != NumOps) {
+    OL[2 + idx] = OL[NumOps - 1];
+    VL[idx] = VL[NumOps - 2 - 1];
   }
 
   // Nuke the last value.
-  OL[NumOps-2].set(nullptr);
-  OL[NumOps-2+1].set(nullptr);
-  setNumHungOffUseOperands(NumOps-2);
+  OL[NumOps - 1].set(nullptr);
+  VL[NumOps - 2 - 1] = nullptr;
+  setNumHungOffUseOperands(NumOps - 1);
 
   return CaseIt(this, idx);
 }
@@ -4139,7 +4142,7 @@ void SwitchInst::growOperands() {
   unsigned NumOps = e*3;
 
   ReservedSpace = NumOps;
-  growHungoffUses(ReservedSpace);
+  growHungoffUses(ReservedSpace, /*WithExtraValues=*/true);
 }
 
 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 IsPhi) {
+void User::allocHungoffUses(unsigned N, bool WithExtraValues) {
   assert(HasHungOffUses && "alloc must have hung off uses");
 
-  static_assert(alignof(Use) >= alignof(BasicBlock *),
+  static_assert(alignof(Use) >= alignof(Value *),
                 "Alignment is insufficient for 'hung-off-uses' pieces");
 
   // Allocate the array of Uses
   size_t size = N * sizeof(Use);
-  if (IsPhi)
-    size += N * sizeof(BasicBlock *);
+  if (WithExtraValues)
+    size += N * sizeof(Value *);
   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 IsPhi) {
+void User::growHungoffUses(unsigned NewNumUses, bool WithExtraValues) {
   assert(HasHungOffUses && "realloc must have hung off uses");
 
   unsigned OldNumUses = getNumOperands();
@@ -77,22 +77,22 @@ void User::growHungoffUses(unsigned NewNumUses, bool IsPhi) {
   assert(NewNumUses > OldNumUses && "realloc must grow num uses");
 
   Use *OldOps = getOperandList();
-  allocHungoffUses(NewNumUses, IsPhi);
+  allocHungoffUses(NewNumUses, WithExtraValues);
   Use *NewOps = getOperandList();
 
   // Now copy from the old operands list to the new one.
   std::copy(OldOps, OldOps + OldNumUses, NewOps);
 
-  // If this is a Phi, then we need to copy the BB pointers too.
-  if (IsPhi) {
+  // If the User has extra values (phi basic blocks, switch case values), then
+  // we need to copy these, too.
+  if (WithExtraValues) {
     auto *OldPtr = reinterpret_cast<char *>(OldOps + OldNumUses);
     auto *NewPtr = reinterpret_cast<char *>(NewOps + NewNumUses);
-    std::copy(OldPtr, OldPtr + (OldNumUses * sizeof(BasicBlock *)), NewPtr);
+    std::copy(OldPtr, OldPtr + (OldNumUses * sizeof(Value *)), NewPtr);
   }
   Use::zap(OldOps, OldOps + OldNumUses, true);
 }
 
-
 // This is a private struct used by `User` to track the co-allocated descriptor
 // section.
 struct DescriptorInfo {