Rename Instruction::isIdenticalTo to Instruction::isIdenticalToWhenDefined,
and introduce a new Instruction::isIdenticalTo which tests for full
identity, including the SubclassOptionalData flags. Also, fix the
Instruction::clone implementations to preserve the SubclassOptionalData
flags. Finally, teach several optimizations how to handle
SubclassOptionalData correctly, given these changes.

This fixes the counterintuitive behavior of isIdenticalTo not comparing
the full value, and clone not returning an identical clone, as well as
some subtle bugs that could be caused by these.

Thanks to Nick Lewycky for reporting this, and for an initial patch!


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@80038 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Transforms/IPO/MergeFunctions.cpp b/lib/Transforms/IPO/MergeFunctions.cpp
index f037c4f..9c8592e 100644
--- a/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/lib/Transforms/IPO/MergeFunctions.cpp
@@ -188,7 +188,8 @@
 isEquivalentOperation(const Instruction *I1, const Instruction *I2) {
   if (I1->getOpcode() != I2->getOpcode() ||
       I1->getNumOperands() != I2->getNumOperands() ||
-      !isEquivalentType(I1->getType(), I2->getType()))
+      !isEquivalentType(I1->getType(), I2->getType()) ||
+      !I1->hasSameSubclassOptionalData(I2))
     return false;
 
   // We have two instructions of identical opcode and #operands.  Check to see
diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp
index 6dd2641..cca428e 100644
--- a/lib/Transforms/Scalar/InstructionCombining.cpp
+++ b/lib/Transforms/Scalar/InstructionCombining.cpp
@@ -11815,12 +11815,16 @@
   if (A == B) return true;
   
   // Test if the values come form identical arithmetic instructions.
+  // This uses isIdenticalToWhenDefined instead of isIdenticalTo because
+  // its only used to compare two uses within the same basic block, which
+  // means that they'll always either have the same value or one of them
+  // will have an undefined value.
   if (isa<BinaryOperator>(A) ||
       isa<CastInst>(A) ||
       isa<PHINode>(A) ||
       isa<GetElementPtrInst>(A))
     if (Instruction *BI = dyn_cast<Instruction>(B))
-      if (cast<Instruction>(A)->isIdenticalTo(BI))
+      if (cast<Instruction>(A)->isIdenticalToWhenDefined(BI))
         return true;
   
   // Otherwise they may not be equivalent.
diff --git a/lib/Transforms/Utils/BasicBlockUtils.cpp b/lib/Transforms/Utils/BasicBlockUtils.cpp
index 3072cee..c3d6194 100644
--- a/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -504,11 +504,15 @@
   // Test if the values are trivially equivalent.
   if (A == B) return true;
   
-  // Test if the values come form identical arithmetic instructions.
+  // Test if the values come from identical arithmetic instructions.
+  // Use isIdenticalToWhenDefined instead of isIdenticalTo because
+  // this function is only used when one address use dominates the
+  // other, which means that they'll always either have the same
+  // value or one of them will have an undefined value.
   if (isa<BinaryOperator>(A) || isa<CastInst>(A) ||
       isa<PHINode>(A) || isa<GetElementPtrInst>(A))
     if (const Instruction *BI = dyn_cast<Instruction>(B))
-      if (cast<Instruction>(A)->isIdenticalTo(BI))
+      if (cast<Instruction>(A)->isIdenticalToWhenDefined(BI))
         return true;
   
   // Otherwise they may not be equivalent.
diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp
index 7b7495e..0938c44 100644
--- a/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -872,7 +872,7 @@
   while (isa<DbgInfoIntrinsic>(I2))
     I2 = BB2_Itr++;
   if (I1->getOpcode() != I2->getOpcode() || isa<PHINode>(I1) ||
-      !I1->isIdenticalTo(I2) ||
+      !I1->isIdenticalToWhenDefined(I2) ||
       (isa<InvokeInst>(I1) && !isSafeToHoistInvoke(BB1, BB2, I1, I2)))
     return false;
 
@@ -891,6 +891,7 @@
     BIParent->getInstList().splice(BI, BB1->getInstList(), I1);
     if (!I2->use_empty())
       I2->replaceAllUsesWith(I1);
+    I1->intersectOptionalDataWith(I2);
     BB2->getInstList().erase(I2);
 
     I1 = BB1_Itr++;
@@ -899,7 +900,8 @@
     I2 = BB2_Itr++;
     while (isa<DbgInfoIntrinsic>(I2))
       I2 = BB2_Itr++;
-  } while (I1->getOpcode() == I2->getOpcode() && I1->isIdenticalTo(I2));
+  } while (I1->getOpcode() == I2->getOpcode() &&
+           I1->isIdenticalToWhenDefined(I2));
 
   return true;
 
diff --git a/lib/VMCore/Instruction.cpp b/lib/VMCore/Instruction.cpp
index e19ad1c..332ecf9 100644
--- a/lib/VMCore/Instruction.cpp
+++ b/lib/VMCore/Instruction.cpp
@@ -168,6 +168,14 @@
 /// identical to the current one.  This means that all operands match and any
 /// extra information (e.g. load is volatile) agree.
 bool Instruction::isIdenticalTo(const Instruction *I) const {
+  return isIdenticalTo(I) &&
+         SubclassOptionalData == I->SubclassOptionalData;
+}
+
+/// isIdenticalToWenDefined - This is like isIdenticalTo, except that it
+/// ignores the SubclassOptionalData flags, which specify conditions
+/// under which the instruction's result is undefined.
+bool Instruction::isIdenticalToWhenDefined(const Instruction *I) const {
   if (getOpcode() != I->getOpcode() ||
       getNumOperands() != I->getNumOperands() ||
       getType() != I->getType())
diff --git a/lib/VMCore/Instructions.cpp b/lib/VMCore/Instructions.cpp
index bbea62b..12a4045 100644
--- a/lib/VMCore/Instructions.cpp
+++ b/lib/VMCore/Instructions.cpp
@@ -154,6 +154,7 @@
     OL[i] = PN.getOperand(i);
     OL[i+1] = PN.getOperand(i+1);
   }
+  SubclassOptionalData = PN.SubclassOptionalData;
 }
 
 PHINode::~PHINode() {
@@ -403,6 +404,7 @@
   Use *InOL = CI.OperandList;
   for (unsigned i = 0, e = CI.getNumOperands(); i != e; ++i)
     OL[i] = InOL[i];
+  SubclassOptionalData = CI.SubclassOptionalData;
 }
 
 void CallInst::addAttribute(unsigned i, Attributes attr) {
@@ -464,6 +466,7 @@
   Use *OL = OperandList, *InOL = II.OperandList;
   for (unsigned i = 0, e = II.getNumOperands(); i != e; ++i)
     OL[i] = InOL[i];
+  SubclassOptionalData = II.SubclassOptionalData;
 }
 
 BasicBlock *InvokeInst::getSuccessorV(unsigned idx) const {
@@ -508,6 +511,7 @@
                    RI.getNumOperands()) {
   if (RI.getNumOperands())
     Op<0>() = RI.Op<0>();
+  SubclassOptionalData = RI.SubclassOptionalData;
 }
 
 ReturnInst::ReturnInst(LLVMContext &C, Value *retVal, Instruction *InsertBefore)
@@ -663,6 +667,7 @@
     Op<-3>() = BI.Op<-3>();
     Op<-2>() = BI.Op<-2>();
   }
+  SubclassOptionalData = BI.SubclassOptionalData;
 }
 
 
@@ -757,12 +762,6 @@
   return getType()->getElementType();
 }
 
-AllocaInst::AllocaInst(const AllocaInst &AI)
-  : AllocationInst(AI.getType()->getElementType(),    
-                   (Value*)AI.getOperand(0), Instruction::Alloca,
-                   AI.getAlignment()) {
-}
-
 /// isStaticAlloca - Return true if this alloca is in the entry block of the
 /// function and is a constant size.  If so, the code generator will fold it
 /// into the prolog/epilog code, so it is basically free.
@@ -775,12 +774,6 @@
   return Parent == &Parent->getParent()->front();
 }
 
-MallocInst::MallocInst(const MallocInst &MI)
-  : AllocationInst(MI.getType()->getElementType(), 
-                   (Value*)MI.getOperand(0), Instruction::Malloc,
-                   MI.getAlignment()) {
-}
-
 //===----------------------------------------------------------------------===//
 //                             FreeInst Implementation
 //===----------------------------------------------------------------------===//
@@ -1048,6 +1041,7 @@
   Use *GEPIOL = GEPI.OperandList;
   for (unsigned i = 0, E = NumOperands; i != E; ++i)
     OL[i] = GEPIOL[i];
+  SubclassOptionalData = GEPI.SubclassOptionalData;
 }
 
 GetElementPtrInst::GetElementPtrInst(Value *Ptr, Value *Idx,
@@ -1211,13 +1205,6 @@
 //                           InsertElementInst Implementation
 //===----------------------------------------------------------------------===//
 
-InsertElementInst::InsertElementInst(const InsertElementInst &IE)
-    : Instruction(IE.getType(), InsertElement,
-                  OperandTraits<InsertElementInst>::op_begin(this), 3) {
-  Op<0>() = IE.Op<0>();
-  Op<1>() = IE.Op<1>();
-  Op<2>() = IE.Op<2>();
-}
 InsertElementInst::InsertElementInst(Value *Vec, Value *Elt, Value *Index,
                                      const Twine &Name,
                                      Instruction *InsertBef)
@@ -1265,15 +1252,6 @@
 //                      ShuffleVectorInst Implementation
 //===----------------------------------------------------------------------===//
 
-ShuffleVectorInst::ShuffleVectorInst(const ShuffleVectorInst &SV) 
-  : Instruction(SV.getType(), ShuffleVector,
-                OperandTraits<ShuffleVectorInst>::op_begin(this),
-                OperandTraits<ShuffleVectorInst>::operands(this)) {
-  Op<0>() = SV.Op<0>();
-  Op<1>() = SV.Op<1>();
-  Op<2>() = SV.Op<2>();
-}
-
 ShuffleVectorInst::ShuffleVectorInst(Value *V1, Value *V2, Value *Mask,
                                      const Twine &Name,
                                      Instruction *InsertBefore)
@@ -1364,6 +1342,7 @@
     Indices(IVI.Indices) {
   Op<0>() = IVI.getOperand(0);
   Op<1>() = IVI.getOperand(1);
+  SubclassOptionalData = IVI.SubclassOptionalData;
 }
 
 InsertValueInst::InsertValueInst(Value *Agg,
@@ -1410,6 +1389,7 @@
 ExtractValueInst::ExtractValueInst(const ExtractValueInst &EVI)
   : UnaryInstruction(EVI.getType(), ExtractValue, EVI.getOperand(0)),
     Indices(EVI.Indices) {
+  SubclassOptionalData = EVI.SubclassOptionalData;
 }
 
 // getIndexedType - Returns the type of the element that would be extracted
@@ -2790,6 +2770,7 @@
     OL[i] = InOL[i];
     OL[i+1] = InOL[i+1];
   }
+  SubclassOptionalData = SI.SubclassOptionalData;
 }
 
 SwitchInst::~SwitchInst() {
@@ -2882,144 +2863,230 @@
 // unit that uses these classes.
 
 GetElementPtrInst *GetElementPtrInst::clone(LLVMContext&) const {
-  return new(getNumOperands()) GetElementPtrInst(*this);
+  GetElementPtrInst *New = new(getNumOperands()) GetElementPtrInst(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 BinaryOperator *BinaryOperator::clone(LLVMContext&) const {
-  return Create(getOpcode(), Op<0>(), Op<1>());
+  BinaryOperator *New = Create(getOpcode(), Op<0>(), Op<1>());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 FCmpInst* FCmpInst::clone(LLVMContext &Context) const {
-  return new FCmpInst(Context, getPredicate(), Op<0>(), Op<1>());
+  FCmpInst *New = new FCmpInst(Context, getPredicate(), Op<0>(), Op<1>());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 ICmpInst* ICmpInst::clone(LLVMContext &Context) const {
-  return new ICmpInst(Context, getPredicate(), Op<0>(), Op<1>());
+  ICmpInst *New = new ICmpInst(Context, getPredicate(), Op<0>(), Op<1>());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 ExtractValueInst *ExtractValueInst::clone(LLVMContext&) const {
-  return new ExtractValueInst(*this);
+  ExtractValueInst *New = new ExtractValueInst(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 InsertValueInst *InsertValueInst::clone(LLVMContext&) const {
-  return new InsertValueInst(*this);
+  InsertValueInst *New = new InsertValueInst(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 MallocInst *MallocInst::clone(LLVMContext&) const {
-  return new MallocInst(*this);
+  MallocInst *New = new MallocInst(getAllocatedType(),
+                                   (Value*)getOperand(0),
+                                   getAlignment());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 AllocaInst *AllocaInst::clone(LLVMContext&) const {
-  return new AllocaInst(*this);
+  AllocaInst *New = new AllocaInst(getAllocatedType(),
+                                   (Value*)getOperand(0),
+                                   getAlignment());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 FreeInst *FreeInst::clone(LLVMContext&) const {
-  return new FreeInst(getOperand(0));
+  FreeInst *New = new FreeInst(getOperand(0));
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 LoadInst *LoadInst::clone(LLVMContext&) const {
-  return new LoadInst(*this);
+  LoadInst *New = new LoadInst(getOperand(0),
+                               Twine(), isVolatile(),
+                               getAlignment());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 StoreInst *StoreInst::clone(LLVMContext&) const {
-  return new StoreInst(*this);
+  StoreInst *New = new StoreInst(getOperand(0), getOperand(1),
+                                 isVolatile(), getAlignment());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *TruncInst::clone(LLVMContext&) const {
-  return new TruncInst(*this);
+  TruncInst *New = new TruncInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *ZExtInst::clone(LLVMContext&) const {
-  return new ZExtInst(*this);
+  ZExtInst *New = new ZExtInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *SExtInst::clone(LLVMContext&) const {
-  return new SExtInst(*this);
+  SExtInst *New = new SExtInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *FPTruncInst::clone(LLVMContext&) const {
-  return new FPTruncInst(*this);
+  FPTruncInst *New = new FPTruncInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *FPExtInst::clone(LLVMContext&) const {
-  return new FPExtInst(*this);
+  FPExtInst *New = new FPExtInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *UIToFPInst::clone(LLVMContext&) const {
-  return new UIToFPInst(*this);
+  UIToFPInst *New = new UIToFPInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *SIToFPInst::clone(LLVMContext&) const {
-  return new SIToFPInst(*this);
+  SIToFPInst *New = new SIToFPInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *FPToUIInst::clone(LLVMContext&) const {
-  return new FPToUIInst(*this);
+  FPToUIInst *New = new FPToUIInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *FPToSIInst::clone(LLVMContext&) const {
-  return new FPToSIInst(*this);
+  FPToSIInst *New = new FPToSIInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *PtrToIntInst::clone(LLVMContext&) const {
-  return new PtrToIntInst(*this);
+  PtrToIntInst *New = new PtrToIntInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *IntToPtrInst::clone(LLVMContext&) const {
-  return new IntToPtrInst(*this);
+  IntToPtrInst *New = new IntToPtrInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CastInst *BitCastInst::clone(LLVMContext&) const {
-  return new BitCastInst(*this);
+  BitCastInst *New = new BitCastInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 CallInst *CallInst::clone(LLVMContext&) const {
-  return new(getNumOperands()) CallInst(*this);
+  CallInst *New = new(getNumOperands()) CallInst(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
-SelectInst *SelectInst::clone(LLVMContext&)   const {
-  return new(getNumOperands()) SelectInst(*this);
+SelectInst *SelectInst::clone(LLVMContext&) const {
+  SelectInst *New = SelectInst::Create(getOperand(0),
+                                       getOperand(1),
+                                       getOperand(2));
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 VAArgInst *VAArgInst::clone(LLVMContext&) const {
-  return new VAArgInst(*this);
+  VAArgInst *New = new VAArgInst(getOperand(0), getType());
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 ExtractElementInst *ExtractElementInst::clone(LLVMContext&) const {
-  return ExtractElementInst::Create(*this);
+  ExtractElementInst *New = ExtractElementInst::Create(getOperand(0),
+                                                       getOperand(1));
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 InsertElementInst *InsertElementInst::clone(LLVMContext&) const {
-  return InsertElementInst::Create(*this);
+  InsertElementInst *New = InsertElementInst::Create(getOperand(0),
+                                                     getOperand(1),
+                                                     getOperand(2));
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 ShuffleVectorInst *ShuffleVectorInst::clone(LLVMContext&) const {
-  return new ShuffleVectorInst(*this);
+  ShuffleVectorInst *New = new ShuffleVectorInst(getOperand(0),
+                                                 getOperand(1),
+                                                 getOperand(2));
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 PHINode *PHINode::clone(LLVMContext&) const {
-  return new PHINode(*this);
+  PHINode *New = new PHINode(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 ReturnInst *ReturnInst::clone(LLVMContext&) const {
-  return new(getNumOperands()) ReturnInst(*this);
+  ReturnInst *New = new(getNumOperands()) ReturnInst(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 BranchInst *BranchInst::clone(LLVMContext&) const {
   unsigned Ops(getNumOperands());
-  return new(Ops, Ops == 1) BranchInst(*this);
+  BranchInst *New = new(Ops, Ops == 1) BranchInst(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 SwitchInst *SwitchInst::clone(LLVMContext&) const {
-  return new SwitchInst(*this);
+  SwitchInst *New = new SwitchInst(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 InvokeInst *InvokeInst::clone(LLVMContext&) const {
-  return new(getNumOperands()) InvokeInst(*this);
+  InvokeInst *New = new(getNumOperands()) InvokeInst(*this);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 UnwindInst *UnwindInst::clone(LLVMContext &C) const {
-  return new UnwindInst(C);
+  UnwindInst *New = new UnwindInst(C);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }
 
 UnreachableInst *UnreachableInst::clone(LLVMContext &C) const {
-  return new UnreachableInst(C);
+  UnreachableInst *New = new UnreachableInst(C);
+  New->SubclassOptionalData = SubclassOptionalData;
+  return New;
 }