IR: Fix ConstantExpr::replaceUsesOfWithOnConstant()

Change `ConstantExpr` to follow the model the other constants are using:
only malloc a replacement if it's going to be used.  This fixes a subtle
bug where if an API user had used `ConstantExpr::get()` already to
create the replacement but hadn't given it any users, we'd delete the
replacement.

This relies on r216015 to thread `OnlyIfReduced` through
`ConstantExpr::getWithOperands()`.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@216016 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/Constants.h b/include/llvm/IR/Constants.h
index 8dbe29273ae4b08cba1e86446bd7ff5eccfaba9a..8591f06c72e5241064f309255ef877c337c336af 100644 (file)
--- a/include/llvm/IR/Constants.h
+++ b/include/llvm/IR/Constants.h
@@ -1142,12 +1142,6 @@ private:
   void setValueSubclassData(unsigned short D) {
     Value::setValueSubclassData(D);
   }
-
-  /// \brief Check whether this can become its replacement.
-  ///
-  /// For use during \a replaceUsesOfWithOnConstant(), check whether we know
-  /// how to turn this into \a Replacement, thereby reducing RAUW traffic.
-  bool canBecomeReplacement(const Constant *Replacement) const;
 };
 
 template <>

diff --git a/lib/IR/Constants.cpp b/lib/IR/Constants.cpp
index e88069a34435a1b493c1540a010c8eb0e550ef94..4cc1e96605fc080361a44b8fa0e15ca7ad34278c 100644 (file)
--- a/lib/IR/Constants.cpp
+++ b/lib/IR/Constants.cpp
@@ -2857,63 +2857,26 @@ void ConstantExpr::replaceUsesOfWithOnConstant(Value *From, Value *ToV,
   Constant *To = cast<Constant>(ToV);
 
   SmallVector<Constant*, 8> NewOps;
+  unsigned NumUpdated = 0;
   for (unsigned i = 0, e = getNumOperands(); i != e; ++i) {
     Constant *Op = getOperand(i);
-    NewOps.push_back(Op == From ? To : Op);
+    if (Op == From) {
+      ++NumUpdated;
+      Op = To;
+    }
+    NewOps.push_back(Op);
   }
+  assert(NumUpdated && "I didn't contain From!");
 
-  Constant *Replacement = getWithOperands(NewOps);
-  assert(Replacement != this && "I didn't contain From!");
-
-  // Check if Replacement has no users (and is the same type).  Ideally, this
-  // check would be done *before* creating Replacement, but threading this
-  // through constant-folding isn't trivial.
-  if (canBecomeReplacement(Replacement)) {
-    // Avoid unnecessary RAUW traffic.
-    auto &ExprConstants = getType()->getContext().pImpl->ExprConstants;
-    ExprConstants.remove(this);
-
-    auto *CE = cast<ConstantExpr>(Replacement);
-    for (unsigned I = 0, E = getNumOperands(); I != E; ++I)
-      // Only set the operands that have actually changed.
-      if (getOperand(I) != CE->getOperand(I))
-        setOperand(I, CE->getOperand(I));
-
-    CE->destroyConstant();
-    ExprConstants.insert(this);
+  if (Constant *C = getWithOperands(NewOps, getType(), true)) {
+    replaceUsesOfWithOnConstantImpl(C);
     return;
   }
 
-  // Everyone using this now uses the replacement.
-  replaceAllUsesWith(Replacement);
-
-  // Delete the old constant!
-  destroyConstant();
-}
-
-bool ConstantExpr::canBecomeReplacement(const Constant *Replacement) const {
-  // If Replacement already has users, use it regardless.
-  if (!Replacement->use_empty())
-    return false;
-
-  // Check for anything that could have changed during constant-folding.
-  if (getValueID() != Replacement->getValueID())
-    return false;
-  const auto *CE = cast<ConstantExpr>(Replacement);
-  if (getOpcode() != CE->getOpcode())
-    return false;
-  if (getNumOperands() != CE->getNumOperands())
-    return false;
-  if (getRawSubclassOptionalData() != CE->getRawSubclassOptionalData())
-    return false;
-  if (isCompare())
-    if (getPredicate() != CE->getPredicate())
-      return false;
-  if (hasIndices())
-    if (getIndices() != CE->getIndices())
-      return false;
-
-  return true;
+  // Update to the new value.
+  if (Constant *C = getContext().pImpl->ExprConstants.replaceOperandsInPlace(
+          NewOps, this, From, To, NumUpdated, U - OperandList))
+    replaceUsesOfWithOnConstantImpl(C);
 }
 
 Instruction *ConstantExpr::getAsInstruction() {

diff --git a/unittests/IR/ConstantsTest.cpp b/unittests/IR/ConstantsTest.cpp
index 52f098e969e06a2d7b1ea37da675113150fa2ccf..f14b09035489701a3363e17919319c6e6c2d9b26 100644 (file)
--- a/unittests/IR/ConstantsTest.cpp
+++ b/unittests/IR/ConstantsTest.cpp
@@ -299,5 +299,28 @@ TEST(ConstantsTest, ConstantArrayReplaceWithConstant) {
   ASSERT_EQ(A01, RefArray->getInitializer());
 }
 
+TEST(ConstantsTest, ConstantExprReplaceWithConstant) {
+  LLVMContext Context;
+  std::unique_ptr<Module> M(new Module("MyModule", Context));
+
+  Type *IntTy = Type::getInt8Ty(Context);
+  Constant *G1 = new GlobalVariable(*M, IntTy, false,
+                                    GlobalValue::ExternalLinkage, nullptr);
+  Constant *G2 = new GlobalVariable(*M, IntTy, false,
+                                    GlobalValue::ExternalLinkage, nullptr);
+  ASSERT_NE(G1, G2);
+
+  Constant *Int1 = ConstantExpr::getPtrToInt(G1, IntTy);
+  Constant *Int2 = ConstantExpr::getPtrToInt(G2, IntTy);
+  ASSERT_NE(Int1, Int2);
+
+  GlobalVariable *Ref =
+      new GlobalVariable(*M, IntTy, false, GlobalValue::ExternalLinkage, Int1);
+  ASSERT_EQ(Int1, Ref->getInitializer());
+
+  G1->replaceAllUsesWith(G2);
+  ASSERT_EQ(Int2, Ref->getInitializer());
+}
+
 }  // end anonymous namespace
 }  // end namespace llvm