add a layer of accessors around the Value::SubClassData member, and use

a convention (shadowing the setter with private forwarding function) to
prevent subclasses from accidentally using it.

This exposed some bogosity in ConstantExprs, which was propaging the
opcode of the constant expr into the NUW/NSW/Exact field in the
getWithOperands/getWithOperandReplaced methods.

llvm-svn: 92239

include/llvm/BasicBlock.h

@@ -239,15 +239,21 @@ public:
 
   /// hasAddressTaken - returns true if there are any uses of this basic block
   /// other than direct branches, switches, etc. to it.
-  bool hasAddressTaken() const { return SubclassData != 0; }
+  bool hasAddressTaken() const { return getSubclassDataFromValue() != 0; }
                      
 private:
   /// AdjustBlockAddressRefCount - BasicBlock stores the number of BlockAddress
   /// objects using it.  This is almost always 0, sometimes one, possibly but
   /// almost never 2, and inconceivably 3 or more.
   void AdjustBlockAddressRefCount(int Amt) {
-    SubclassData += Amt;
-    assert((int)(signed char)SubclassData >= 0 && "Refcount wrap-around");
+    setValueSubclassData(getSubclassDataFromValue()+Amt);
+    assert((int)(signed char)getSubclassDataFromValue() >= 0 &&
+           "Refcount wrap-around");
+  }
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // any future subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
   }
 };
 

include/llvm/Constants.h

@@ -605,7 +605,7 @@ protected:
   ConstantExpr(const Type *ty, unsigned Opcode, Use *Ops, unsigned NumOps)
     : Constant(ty, ConstantExprVal, Ops, NumOps) {
     // Operation type (an Instruction opcode) is stored as the SubclassData.
-    SubclassData = Opcode;
+    setValueSubclassData(Opcode);
   }
 
   // These private methods are used by the type resolution code to create
@@ -814,7 +814,7 @@ public:
   virtual bool isNullValue() const { return false; }
 
   /// getOpcode - Return the opcode at the root of this constant expression
-  unsigned getOpcode() const { return SubclassData; }
+  unsigned getOpcode() const { return getSubclassDataFromValue(); }
 
   /// getPredicate - Return the ICMP or FCMP predicate value. Assert if this is
   /// not an ICMP or FCMP constant expression.
@@ -847,6 +847,13 @@ public:
   static inline bool classof(const Value *V) {
     return V->getValueID() == ConstantExprVal;
   }
+  
+private:
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 template <>

include/llvm/Function.h

@@ -87,6 +87,9 @@ private:
   ValueSymbolTable *SymTab;               ///< Symbol table of args/instructions
   AttrListPtr AttributeList;              ///< Parameter attributes
 
+  // HasLazyArguments is stored in Value::SubclassData.
+  /*bool HasLazyArguments;*/
+                   
   // The Calling Convention is stored in Value::SubclassData.
   /*CallingConv::ID CallingConvention;*/
 
@@ -99,7 +102,7 @@ private:
   /// needs it.  The hasLazyArguments predicate returns true if the arg list
   /// hasn't been set up yet.
   bool hasLazyArguments() const {
-    return SubclassData & 1;
+    return getSubclassDataFromValue() & 1;
   }
   void CheckLazyArguments() const {
     if (hasLazyArguments())
@@ -156,10 +159,11 @@ public:
   /// calling convention of this function.  The enum values for the known
   /// calling conventions are defined in CallingConv.h.
   CallingConv::ID getCallingConv() const {
-    return static_cast<CallingConv::ID>(SubclassData >> 1);
+    return static_cast<CallingConv::ID>(getSubclassDataFromValue() >> 1);
   }
   void setCallingConv(CallingConv::ID CC) {
-    SubclassData = (SubclassData & 1) | (static_cast<unsigned>(CC) << 1);
+    setValueSubclassData((getSubclassDataFromValue() & 1) |
+                         (static_cast<unsigned>(CC) << 1));
   }
   
   /// getAttributes - Return the attribute list for this Function.
@@ -407,6 +411,12 @@ public:
   /// hasAddressTaken - returns true if there are any uses of this function
   /// other than direct calls or invokes to it.
   bool hasAddressTaken() const;
+private:
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 inline ValueSymbolTable *

include/llvm/InstrTypes.h

@@ -732,10 +732,12 @@ public:
   }
 
   /// @brief Return the predicate for this instruction.
-  Predicate getPredicate() const { return Predicate(SubclassData); }
+  Predicate getPredicate() const {
+    return Predicate(getSubclassDataFromValue());
+  }
 
   /// @brief Set the predicate for this instruction to the specified value.
-  void setPredicate(Predicate P) { SubclassData = P; }
+  void setPredicate(Predicate P) { setValueSubclassData(P); }
 
   static bool isFPPredicate(Predicate P) {
     return P >= FIRST_FCMP_PREDICATE && P <= LAST_FCMP_PREDICATE;
@@ -856,6 +858,12 @@ public:
     }
     return Type::getInt1Ty(opnd_type->getContext());
   }
+private:
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 

include/llvm/Instructions.h

@@ -82,7 +82,9 @@ public:
   /// getAlignment - Return the alignment of the memory that is being allocated
   /// by the instruction.
   ///
-  unsigned getAlignment() const { return (1u << SubclassData) >> 1; }
+  unsigned getAlignment() const {
+    return (1u << getSubclassDataFromValue()) >> 1;
+  }
   void setAlignment(unsigned Align);
 
   /// isStaticAlloca - Return true if this alloca is in the entry block of the
@@ -98,6 +100,12 @@ public:
   static inline bool classof(const Value *V) {
     return isa<Instruction>(V) && classof(cast<Instruction>(V));
   }
+private:
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 
@@ -134,18 +142,18 @@ public:
   /// isVolatile - Return true if this is a load from a volatile memory
   /// location.
   ///
-  bool isVolatile() const { return SubclassData & 1; }
+  bool isVolatile() const { return getSubclassDataFromValue() & 1; }
 
   /// setVolatile - Specify whether this is a volatile load or not.
   ///
   void setVolatile(bool V) {
-    SubclassData = (SubclassData & ~1) | (V ? 1 : 0);
+    setValueSubclassData((getSubclassDataFromValue() & ~1) | (V ? 1 : 0));
   }
 
   /// getAlignment - Return the alignment of the access that is being performed
   ///
   unsigned getAlignment() const {
-    return (1 << (SubclassData>>1)) >> 1;
+    return (1 << (getSubclassDataFromValue() >> 1)) >> 1;
   }
 
   void setAlignment(unsigned Align);
@@ -167,6 +175,12 @@ public:
   static inline bool classof(const Value *V) {
     return isa<Instruction>(V) && classof(cast<Instruction>(V));
   }
+private:
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 
@@ -200,12 +214,12 @@ public:
   /// isVolatile - Return true if this is a load from a volatile memory
   /// location.
   ///
-  bool isVolatile() const { return SubclassData & 1; }
+  bool isVolatile() const { return getSubclassDataFromValue() & 1; }
 
   /// setVolatile - Specify whether this is a volatile load or not.
   ///
   void setVolatile(bool V) {
-    SubclassData = (SubclassData & ~1) | (V ? 1 : 0);
+    setValueSubclassData((getSubclassDataFromValue() & ~1) | (V ? 1 : 0));
   }
 
   /// Transparently provide more efficient getOperand methods.
@@ -214,7 +228,7 @@ public:
   /// getAlignment - Return the alignment of the access that is being performed
   ///
   unsigned getAlignment() const {
-    return (1 << (SubclassData>>1)) >> 1;
+    return (1 << (getSubclassDataFromValue() >> 1)) >> 1;
   }
 
   void setAlignment(unsigned Align);
@@ -235,6 +249,12 @@ public:
   static inline bool classof(const Value *V) {
     return isa<Instruction>(V) && classof(cast<Instruction>(V));
   }
+private:
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 template <>
@@ -675,7 +695,7 @@ public:
   /// (e.g. ult).
   /// @brief Swap operands and adjust predicate.
   void swapOperands() {
-    SubclassData = getSwappedPredicate();
+    setPredicate(getSwappedPredicate());
     Op<0>().swap(Op<1>());
   }
 
@@ -761,18 +781,18 @@ public:
   /// @returns true if the predicate of this instruction is EQ or NE.
   /// @brief Determine if this is an equality predicate.
   bool isEquality() const {
-    return SubclassData == FCMP_OEQ || SubclassData == FCMP_ONE ||
-           SubclassData == FCMP_UEQ || SubclassData == FCMP_UNE;
+    return getPredicate() == FCMP_OEQ || getPredicate() == FCMP_ONE ||
+           getPredicate() == FCMP_UEQ || getPredicate() == FCMP_UNE;
   }
 
   /// @returns true if the predicate of this instruction is commutative.
   /// @brief Determine if this is a commutative predicate.
   bool isCommutative() const {
     return isEquality() ||
-           SubclassData == FCMP_FALSE ||
-           SubclassData == FCMP_TRUE ||
-           SubclassData == FCMP_ORD ||
-           SubclassData == FCMP_UNO;
+           getPredicate() == FCMP_FALSE ||
+           getPredicate() == FCMP_TRUE ||
+           getPredicate() == FCMP_ORD ||
+           getPredicate() == FCMP_UNO;
   }
 
   /// @returns true if the predicate is relational (not EQ or NE).
@@ -785,7 +805,7 @@ public:
   /// (e.g. ult).
   /// @brief Swap operands and adjust predicate.
   void swapOperands() {
-    SubclassData = getSwappedPredicate();
+    setPredicate(getSwappedPredicate());
     Op<0>().swap(Op<1>());
   }
 
@@ -799,15 +819,12 @@ public:
   }
 };
 
-//===----------------------------------------------------------------------===//
-//                                 CallInst Class
 //===----------------------------------------------------------------------===//
 /// CallInst - This class represents a function call, abstracting a target
 /// machine's calling convention.  This class uses low bit of the SubClassData
 /// field to indicate whether or not this is a tail call.  The rest of the bits
 /// hold the calling convention of the call.
 ///
-
 class CallInst : public Instruction {
   AttrListPtr AttributeList; ///< parameter attributes for call
   CallInst(const CallInst &CI);
@@ -912,9 +929,9 @@ public:
 
   ~CallInst();
 
-  bool isTailCall() const           { return SubclassData & 1; }
+  bool isTailCall() const { return getSubclassDataFromValue() & 1; }
   void setTailCall(bool isTC = true) {
-    SubclassData = (SubclassData & ~1) | unsigned(isTC);
+    setValueSubclassData((getSubclassDataFromValue() & ~1) | unsigned(isTC));
   }
 
   /// Provide fast operand accessors
@@ -923,10 +940,11 @@ public:
   /// getCallingConv/setCallingConv - Get or set the calling convention of this
   /// function call.
   CallingConv::ID getCallingConv() const {
-    return static_cast<CallingConv::ID>(SubclassData >> 1);
+    return static_cast<CallingConv::ID>(getSubclassDataFromValue() >> 1);
   }
   void setCallingConv(CallingConv::ID CC) {
-    SubclassData = (SubclassData & 1) | (static_cast<unsigned>(CC) << 1);
+    setValueSubclassData((getSubclassDataFromValue() & 1) |
+                         (static_cast<unsigned>(CC) << 1));
   }
 
   /// getAttributes - Return the parameter attributes for this call.
@@ -1024,6 +1042,12 @@ public:
   static inline bool classof(const Value *V) {
     return isa<Instruction>(V) && classof(cast<Instruction>(V));
   }
+private:
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 template <>
@@ -2401,10 +2425,10 @@ public:
   /// getCallingConv/setCallingConv - Get or set the calling convention of this
   /// function call.
   CallingConv::ID getCallingConv() const {
-    return static_cast<CallingConv::ID>(SubclassData);
+    return static_cast<CallingConv::ID>(getSubclassDataFromValue());
   }
   void setCallingConv(CallingConv::ID CC) {
-    SubclassData = static_cast<unsigned>(CC);
+    setValueSubclassData(static_cast<unsigned>(CC));
   }
 
   /// getAttributes - Return the parameter attributes for this invoke.
@@ -2528,6 +2552,12 @@ private:
   virtual BasicBlock *getSuccessorV(unsigned idx) const;
   virtual unsigned getNumSuccessorsV() const;
   virtual void setSuccessorV(unsigned idx, BasicBlock *B);
+
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 template <>

include/llvm/Metadata.h

@@ -125,7 +125,9 @@ public:
   /// Note: MDNodes are designated as function-local when created, and keep
   ///       that designation even if their operands are modified to no longer
   ///       refer to function-local IR.
-  bool isFunctionLocal() const { return SubclassData & FunctionLocalBit; }
+  bool isFunctionLocal() const {
+    return (getSubclassDataFromValue() & FunctionLocalBit) != 0;
+  }
 
   /// Profile - calculate a unique identifier for this MDNode to collapse
   /// duplicates
@@ -136,6 +138,12 @@ public:
   static bool classof(const Value *V) {
     return V->getValueID() == MDNodeVal;
   }
+private:
+  // Shadow Value::setValueSubclassData with a private forwarding method so that
+  // any future subclasses cannot accidentally use it.
+  void setValueSubclassData(unsigned short D) {
+    Value::setValueSubclassData(D);
+  }
 };
 
 //===----------------------------------------------------------------------===//

include/llvm/Value.h

@@ -57,7 +57,7 @@ class MetadataContextImpl;
 ///
 /// Every value has a "use list" that keeps track of which other Values are
 /// using this Value.  A Value can also have an arbitrary number of ValueHandle
-/// objects that watch it and listen to RAUW and Destroy events see
+/// objects that watch it and listen to RAUW and Destroy events.  See
 /// llvm/Support/ValueHandle.h for details.
 ///
 /// @brief LLVM Value Representation
@@ -71,11 +71,12 @@ protected:
   /// interpretation.
   unsigned char SubclassOptionalData : 7;
 
+private:
   /// SubclassData - This member is defined by this class, but is not used for
   /// anything.  Subclasses can use it to hold whatever state they find useful.
   /// This field is initialized to zero by the ctor.
   unsigned short SubclassData;
-private:
+
   PATypeHolder VTy;
   Use *UseList;
 
@@ -300,6 +301,10 @@ public:
                                 const BasicBlock *PredBB) const{
     return const_cast<Value*>(this)->DoPHITranslation(CurBB, PredBB);
   }
+  
+protected:
+  unsigned short getSubclassDataFromValue() const { return SubclassData; }
+  void setValueSubclassData(unsigned short D) { SubclassData = D; }
 };
 
 inline raw_ostream &operator<<(raw_ostream &OS, const Value &V) {

lib/VMCore/Constants.cpp

@@ -763,14 +763,14 @@ ConstantExpr::getWithOperandReplaced(unsigned OpNo, Constant *Op) const {
         ConstantExpr::getGetElementPtr(Op, &Ops[0], Ops.size());
     Ops[OpNo-1] = Op;
     return cast<GEPOperator>(this)->isInBounds() ?
-      ConstantExpr::getInBoundsGetElementPtr(getOperand(0), &Ops[0], Ops.size()) :
+      ConstantExpr::getInBoundsGetElementPtr(getOperand(0), &Ops[0],Ops.size()):
       ConstantExpr::getGetElementPtr(getOperand(0), &Ops[0], Ops.size());
   }
   default:
     assert(getNumOperands() == 2 && "Must be binary operator?");
     Op0 = (OpNo == 0) ? Op : getOperand(0);
     Op1 = (OpNo == 1) ? Op : getOperand(1);
-    return ConstantExpr::get(getOpcode(), Op0, Op1, SubclassData);
+    return ConstantExpr::get(getOpcode(), Op0, Op1, SubclassOptionalData);
   }
 }
 
@@ -820,7 +820,7 @@ getWithOperands(Constant* const *Ops, unsigned NumOps) const {
     return ConstantExpr::getCompare(getPredicate(), Ops[0], Ops[1]);
   default:
     assert(getNumOperands() == 2 && "Must be binary operator?");
-    return ConstantExpr::get(getOpcode(), Ops[0], Ops[1], SubclassData);
+    return ConstantExpr::get(getOpcode(), Ops[0], Ops[1], SubclassOptionalData);
   }
 }
 
@@ -2196,7 +2196,7 @@ void ConstantExpr::replaceUsesOfWithOnConstant(Value *From, Value *ToV,
     Constant *C2 = getOperand(1);
     if (C1 == From) C1 = To;
     if (C2 == From) C2 = To;
-    Replacement = ConstantExpr::get(getOpcode(), C1, C2, SubclassData);
+    Replacement = ConstantExpr::get(getOpcode(), C1, C2, SubclassOptionalData);
   } else {
     llvm_unreachable("Unknown ConstantExpr type!");
     return;

lib/VMCore/Function.cpp

@@ -160,7 +160,7 @@ Function::Function(const FunctionType *Ty, LinkageTypes Linkage,
 
   // If the function has arguments, mark them as lazily built.
   if (Ty->getNumParams())
-    SubclassData = 1;   // Set the "has lazy arguments" bit.
+    setValueSubclassData(1);   // Set the "has lazy arguments" bit.
   
   // Make sure that we get added to a function
   LeakDetector::addGarbageObject(this);
@@ -195,7 +195,8 @@ void Function::BuildLazyArguments() const {
   }
   
   // Clear the lazy arguments bit.
-  const_cast<Function*>(this)->SubclassData &= ~1;
+  unsigned SDC = getSubclassDataFromValue();
+  const_cast<Function*>(this)->setValueSubclassData(SDC &= ~1);
 }
 
 size_t Function::arg_size() const {

lib/VMCore/Instructions.cpp

@@ -413,7 +413,9 @@ CallInst::CallInst(const CallInst &CI)
                 OperandTraits<CallInst>::op_end(this) - CI.getNumOperands(),
                 CI.getNumOperands()) {
   setAttributes(CI.getAttributes());
-  SubclassData = CI.SubclassData;
+  setTailCall(CI.isTailCall());
+  setCallingConv(CI.getCallingConv());
+    
   Use *OL = OperandList;
   Use *InOL = CI.OperandList;
   for (unsigned i = 0, e = CI.getNumOperands(); i != e; ++i)
@@ -637,7 +639,7 @@ InvokeInst::InvokeInst(const InvokeInst &II)
                    - II.getNumOperands(),
                    II.getNumOperands()) {
   setAttributes(II.getAttributes());
-  SubclassData = II.SubclassData;
+  setCallingConv(II.getCallingConv());
   Use *OL = OperandList, *InOL = II.OperandList;
   for (unsigned i = 0, e = II.getNumOperands(); i != e; ++i)
     OL[i] = InOL[i];
@@ -957,7 +959,7 @@ AllocaInst::~AllocaInst() {
 
 void AllocaInst::setAlignment(unsigned Align) {
   assert((Align & (Align-1)) == 0 && "Alignment is not a power of 2!");
-  SubclassData = Log2_32(Align) + 1;
+  setValueSubclassData(Log2_32(Align) + 1);
   assert(getAlignment() == Align && "Alignment representation error!");
 }
 
@@ -1092,7 +1094,8 @@ LoadInst::LoadInst(Value *Ptr, const char *Name, bool isVolatile,
 
 void LoadInst::setAlignment(unsigned Align) {
   assert((Align & (Align-1)) == 0 && "Alignment is not a power of 2!");
-  SubclassData = (SubclassData & 1) | ((Log2_32(Align)+1)<<1);
+  setValueSubclassData((getSubclassDataFromValue() & 1) |
+                       ((Log2_32(Align)+1)<<1));
 }
 
 //===----------------------------------------------------------------------===//
@@ -1187,7 +1190,8 @@ StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile,
 
 void StoreInst::setAlignment(unsigned Align) {
   assert((Align & (Align-1)) == 0 && "Alignment is not a power of 2!");
-  SubclassData = (SubclassData & 1) | ((Log2_32(Align)+1)<<1);
+  setValueSubclassData((getSubclassDataFromValue() & 1) |
+                       ((Log2_32(Align)+1) << 1));
 }
 
 //===----------------------------------------------------------------------===//
@@ -2720,7 +2724,7 @@ CmpInst::CmpInst(const Type *ty, OtherOps op, unsigned short predicate,
                 InsertBefore) {
     Op<0>() = LHS;
     Op<1>() = RHS;
-  SubclassData = predicate;
+  setPredicate((Predicate)predicate);
   setName(Name);
 }
 
@@ -2733,7 +2737,7 @@ CmpInst::CmpInst(const Type *ty, OtherOps op, unsigned short predicate,
                 InsertAtEnd) {
   Op<0>() = LHS;
   Op<1>() = RHS;
-  SubclassData = predicate;
+  setPredicate((Predicate)predicate);
   setName(Name);
 }
 

lib/VMCore/Metadata.cpp

@@ -103,7 +103,7 @@ MDNode::MDNode(LLVMContext &C, Value *const *Vals, unsigned NumVals,
     Operands[i].set(Vals[i], this);
     
   if (isFunctionLocal)
-    SubclassData |= FunctionLocalBit;
+    setValueSubclassData(getSubclassDataFromValue() | FunctionLocalBit);
 }
 
 MDNode *MDNode::get(LLVMContext &Context, Value*const* Vals, unsigned NumVals,