[llvm] r257751 - Revert "Assert that we have all use/users in the getters."

Michael Zolotukhin via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 14 14:19:05 PST 2016


Sorry for including hash instead of revision and not mentioning this in the commit message: the reason for reverting is PR25919, which leads to breakage in LLVM builds on our bots.

Michael
> On Jan 14, 2016, at 1:02 AM, Michael Zolotukhin via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: mzolotukhin
> Date: Thu Jan 14 03:02:45 2016
> New Revision: 257751
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=257751&view=rev
> Log:
> Revert "Assert that we have all use/users in the getters."
> 
> This reverts commit fdb838f3f8a8b6896bbbd5285555874eb3b748eb.
> 
> Modified:
>    llvm/trunk/include/llvm/IR/Module.h
>    llvm/trunk/include/llvm/IR/Value.h
>    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>    llvm/trunk/lib/IR/Module.cpp
>    llvm/trunk/lib/IR/Value.cpp
>    llvm/trunk/lib/IR/Verifier.cpp
>    llvm/trunk/tools/llvm-extract/llvm-extract.cpp
> 
> Modified: llvm/trunk/include/llvm/IR/Module.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Module.h?rev=257751&r1=257750&r2=257751&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/Module.h (original)
> +++ llvm/trunk/include/llvm/IR/Module.h Thu Jan 14 03:02:45 2016
> @@ -439,7 +439,6 @@ public:
>   void setMaterializer(GVMaterializer *GVM);
>   /// Retrieves the GVMaterializer, if any, for this Module.
>   GVMaterializer *getMaterializer() const { return Materializer.get(); }
> -  bool isMaterialized() const { return !getMaterializer(); }
> 
>   /// Make sure the GlobalValue is fully read. If the module is corrupt, this
>   /// returns true and fills in the optional string with information about the
> @@ -447,6 +446,7 @@ public:
>   std::error_code materialize(GlobalValue *GV);
> 
>   /// Make sure all GlobalValues in this Module are fully read and clear the
> +  /// Materializer. If the module is corrupt, this DOES NOT clear the old
>   /// Materializer.
>   std::error_code materializeAll();
> 
> 
> Modified: llvm/trunk/include/llvm/IR/Value.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Value.h?rev=257751&r1=257750&r2=257751&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/Value.h (original)
> +++ llvm/trunk/include/llvm/IR/Value.h Thu Jan 14 03:02:45 2016
> @@ -273,91 +273,38 @@ public:
>   //----------------------------------------------------------------------
>   // Methods for handling the chain of uses of this Value.
>   //
> -  // Materializing a function can introduce new uses, so these methods come in
> -  // two variants:
> -  // The methods that start with materialized_ check the uses that are
> -  // currently known given which functions are materialized. Be very careful
> -  // when using them since you might not get all uses.
> -  // The methods that don't start with materialized_ assert that modules is
> -  // fully materialized.
> -#ifdef NDEBUG
> -  void assertModuleIsMaterialized() const {}
> -#else
> -  void assertModuleIsMaterialized() const;
> -#endif
> -
> -  bool use_empty() const {
> -    assertModuleIsMaterialized();
> -    return UseList == nullptr;
> -  }
> +  bool use_empty() const { return UseList == nullptr; }
> 
>   typedef use_iterator_impl<Use> use_iterator;
>   typedef use_iterator_impl<const Use> const_use_iterator;
> -  use_iterator materialized_use_begin() { return use_iterator(UseList); }
> -  const_use_iterator materialized_use_begin() const {
> -    return const_use_iterator(UseList);
> -  }
> -  use_iterator use_begin() {
> -    assertModuleIsMaterialized();
> -    return materialized_use_begin();
> -  }
> -  const_use_iterator use_begin() const {
> -    assertModuleIsMaterialized();
> -    return materialized_use_begin();
> -  }
> +  use_iterator use_begin() { return use_iterator(UseList); }
> +  const_use_iterator use_begin() const { return const_use_iterator(UseList); }
>   use_iterator use_end() { return use_iterator(); }
>   const_use_iterator use_end() const { return const_use_iterator(); }
> -  iterator_range<use_iterator> materialized_uses() {
> -    return make_range(materialized_use_begin(), use_end());
> -  }
> -  iterator_range<const_use_iterator> materialized_uses() const {
> -    return make_range(materialized_use_begin(), use_end());
> -  }
>   iterator_range<use_iterator> uses() {
> -    assertModuleIsMaterialized();
> -    return materialized_uses();
> +    return make_range(use_begin(), use_end());
>   }
>   iterator_range<const_use_iterator> uses() const {
> -    assertModuleIsMaterialized();
> -    return materialized_uses();
> +    return make_range(use_begin(), use_end());
>   }
> 
> -  bool user_empty() const {
> -    assertModuleIsMaterialized();
> -    return UseList == nullptr;
> -  }
> +  bool user_empty() const { return UseList == nullptr; }
> 
>   typedef user_iterator_impl<User> user_iterator;
>   typedef user_iterator_impl<const User> const_user_iterator;
> -  user_iterator materialized_user_begin() { return user_iterator(UseList); }
> -  const_user_iterator materialized_user_begin() const {
> -    return const_user_iterator(UseList);
> -  }
> -  user_iterator user_begin() {
> -    assertModuleIsMaterialized();
> -    return materialized_user_begin();
> -  }
> +  user_iterator user_begin() { return user_iterator(UseList); }
>   const_user_iterator user_begin() const {
> -    assertModuleIsMaterialized();
> -    return materialized_user_begin();
> +    return const_user_iterator(UseList);
>   }
>   user_iterator user_end() { return user_iterator(); }
>   const_user_iterator user_end() const { return const_user_iterator(); }
> -  User *user_back() {
> -    assertModuleIsMaterialized();
> -    return *materialized_user_begin();
> -  }
> -  const User *user_back() const {
> -    assertModuleIsMaterialized();
> -    return *materialized_user_begin();
> -  }
> +  User *user_back() { return *user_begin(); }
> +  const User *user_back() const { return *user_begin(); }
>   iterator_range<user_iterator> users() {
> -    assertModuleIsMaterialized();
> -    return make_range(materialized_user_begin(), user_end());
> +    return make_range(user_begin(), user_end());
>   }
>   iterator_range<const_user_iterator> users() const {
> -    assertModuleIsMaterialized();
> -    return make_range(materialized_user_begin(), user_end());
> +    return make_range(user_begin(), user_end());
>   }
> 
>   /// \brief Return true if there is exactly one user of this value.
> 
> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=257751&r1=257750&r2=257751&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Thu Jan 14 03:02:45 2016
> @@ -3022,7 +3022,7 @@ std::error_code BitcodeReader::parseUseL
>         V = ValueList[ID];
>       unsigned NumUses = 0;
>       SmallDenseMap<const Use *, unsigned, 16> Order;
> -      for (const Use &U : V->materialized_uses()) {
> +      for (const Use &U : V->uses()) {
>         if (++NumUses > Record.size())
>           break;
>         Order[&U] = Record[NumUses - 1];
> @@ -5267,8 +5267,7 @@ std::error_code BitcodeReader::materiali
> 
>   // Upgrade any old intrinsic calls in the function.
>   for (auto &I : UpgradedIntrinsics) {
> -    for (auto UI = I.first->materialized_user_begin(), UE = I.first->user_end();
> -         UI != UE;) {
> +    for (auto UI = I.first->user_begin(), UE = I.first->user_end(); UI != UE;) {
>       User *U = *UI;
>       ++UI;
>       if (CallInst *CI = dyn_cast<CallInst>(U))
> 
> Modified: llvm/trunk/lib/IR/Module.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Module.cpp?rev=257751&r1=257750&r2=257751&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Module.cpp (original)
> +++ llvm/trunk/lib/IR/Module.cpp Thu Jan 14 03:02:45 2016
> @@ -394,8 +394,10 @@ std::error_code Module::materialize(Glob
> std::error_code Module::materializeAll() {
>   if (!Materializer)
>     return std::error_code();
> -  std::unique_ptr<GVMaterializer> M = std::move(Materializer);
> -  return M->materializeModule();
> +  if (std::error_code EC = Materializer->materializeModule())
> +    return EC;
> +  Materializer.reset();
> +  return std::error_code();
> }
> 
> std::error_code Module::materializeMetadata() {
> 
> Modified: llvm/trunk/lib/IR/Value.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Value.cpp?rev=257751&r1=257750&r2=257751&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Value.cpp (original)
> +++ llvm/trunk/lib/IR/Value.cpp Thu Jan 14 03:02:45 2016
> @@ -314,16 +314,6 @@ void Value::takeName(Value *V) {
> }
> 
> #ifndef NDEBUG
> -void Value::assertModuleIsMaterialized() const {
> -  const GlobalValue *GV = dyn_cast<GlobalValue>(this);
> -  if (!GV)
> -    return;
> -  const Module *M = GV->getParent();
> -  if (!M)
> -    return;
> -  assert(M->isMaterialized());
> -}
> -
> static bool contains(SmallPtrSetImpl<ConstantExpr *> &Cache, ConstantExpr *Expr,
>                      Constant *C) {
>   if (!Cache.insert(Expr).second)
> 
> Modified: llvm/trunk/lib/IR/Verifier.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=257751&r1=257750&r2=257751&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Verifier.cpp (original)
> +++ llvm/trunk/lib/IR/Verifier.cpp Thu Jan 14 03:02:45 2016
> @@ -1901,9 +1901,7 @@ void Verifier::visitFunction(const Funct
> 
>   // If this function is actually an intrinsic, verify that it is only used in
>   // direct call/invokes, never having its "address taken".
> -  // Only do this if the module is materialized, otherwise we don't have all the
> -  // uses.
> -  if (F.getIntrinsicID() && F.getParent()->isMaterialized()) {
> +  if (F.getIntrinsicID()) {
>     const User *U;
>     if (F.hasAddressTaken(&U))
>       Assert(0, "Invalid user of intrinsic instruction!", U);
> 
> Modified: llvm/trunk/tools/llvm-extract/llvm-extract.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-extract/llvm-extract.cpp?rev=257751&r1=257750&r2=257751&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llvm-extract/llvm-extract.cpp (original)
> +++ llvm/trunk/tools/llvm-extract/llvm-extract.cpp Thu Jan 14 03:02:45 2016
> @@ -242,22 +242,13 @@ int main(int argc, char **argv) {
>     }
>   }
> 
> -  {
> -    std::vector<GlobalValue *> Gvs(GVs.begin(), GVs.end());
> -    legacy::PassManager Extract;
> -    Extract.add(createGVExtractionPass(Gvs, DeleteFn));
> -    Extract.run(*M);
> -
> -    // Now that we have all the GVs we want, mark the module as fully
> -    // materialized.
> -    // FIXME: should the GVExtractionPass handle this?
> -    M->materializeAll();
> -  }
> -
>   // In addition to deleting all other functions, we also want to spiff it
>   // up a little bit.  Do this now.
>   legacy::PassManager Passes;
> 
> +  std::vector<GlobalValue*> Gvs(GVs.begin(), GVs.end());
> +
> +  Passes.add(createGVExtractionPass(Gvs, DeleteFn));
>   if (!DeleteFn)
>     Passes.add(createGlobalDCEPass());           // Delete unreachable globals
>   Passes.add(createStripDeadDebugInfoPass());    // Remove dead debug info
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list