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

6 files changed, 157 insertions, 71 deletions

diff --git a/lib/Transforms/IPO/MergeFunctions.cpp b/lib/Transforms/IPO/MergeFunctions.cpp
index f037c4fa20..9c8592e8eb 100644
--- a/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/lib/Transforms/IPO/MergeFunctions.cpp
@@ -188,7 +188,8 @@ static bool
 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 6dd2641ad4..cca428e7c4 100644
--- a/lib/Transforms/Scalar/InstructionCombining.cpp
+++ b/lib/Transforms/Scalar/InstructionCombining.cpp
@@ -11815,12 +11815,16 @@ static bool equivalentAddressValues(Value *A, Value *B) {
   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 3072cee61c..c3d6194801 100644
--- a/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -504,11 +504,15 @@ static bool AreEquivalentAddressValues(const Value *A, const Value *B) {
   // 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 7b7495e665..0938c44c96 100644
--- a/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -872,7 +872,7 @@ static bool HoistThenElseCodeToIf(BranchInst *BI) {
   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 @@ static bool HoistThenElseCodeToIf(BranchInst *BI) {
     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 @@ static bool HoistThenElseCodeToIf(BranchInst *BI) {
     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 e19ad1c16f..332ecf9782 100644
--- a/lib/VMCore/Instruction.cpp
+++ b/lib/VMCore/Instruction.cpp
@@ -168,6 +168,14 @@ const char *Instruction::getOpcodeName(unsigned OpCode) {
 /// 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 bbea62bded..12a4045a14 100644
--- a/lib/VMCore/Instructions.cpp
+++ b/lib/VMCore/Instructions.cpp
@@ -154,6 +154,7 @@ PHINode::PHINode(const PHINode &PN)
     OL[i] = PN.getOperand(i);
     OL[i+1] = PN.getOperand(i+1);
   }
+  SubclassOptionalData = PN.SubclassOptionalData;
 }
 
 PHINode::~PHINode() {
@@ -403,6 +404,7 @@ CallInst::CallInst(const CallInst &CI)
   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 @@ InvokeInst::InvokeInst(const InvokeInst &II)
   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 @@ ReturnInst::ReturnInst(const ReturnInst &RI)
                    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 @@ BranchInst::BranchInst(const BranchInst &BI) :
     Op<-3>() = BI.Op<-3>();
     Op<-2>() = BI.Op<-2>();
   }
+  SubclassOptionalData = BI.SubclassOptionalData;
 }
 
 
@@ -757,12 +762,6 @@ const Type *AllocationInst::getAllocatedType() const {
   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 @@ bool AllocaInst::isStaticAlloca() const {
   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 @@ GetElementPtrInst::GetElementPtrInst(const GetElementPtrInst &GEPI)
   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 @@ bool ExtractElementInst::isValidOperands(const Value *Val, const Value *Index) {
 //                           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 @@ bool InsertElementInst::isValidOperands(const Value *Vec, const Value *Elt,
 //                      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 @@ InsertValueInst::InsertValueInst(const InsertValueInst &IVI)
     Indices(IVI.Indices) {
   Op<0>() = IVI.getOperand(0);
   Op<1>() = IVI.getOperand(1);
+  SubclassOptionalData = IVI.SubclassOptionalData;
 }
 
 InsertValueInst::InsertValueInst(Value *Agg,
@@ -1410,6 +1389,7 @@ void ExtractValueInst::init(unsigned Idx, const Twine &Name) {
 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 @@ SwitchInst::SwitchInst(const SwitchInst &SI)
     OL[i] = InOL[i];
     OL[i+1] = InOL[i+1];
   }
+  SubclassOptionalData = SI.SubclassOptionalData;
 }
 
 SwitchInst::~SwitchInst() {
@@ -2882,144 +2863,230 @@ void SwitchInst::setSuccessorV(unsigned idx, BasicBlock *B) {
 // 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;
 }