[PATCH] D46425: Clear converters map after X86 Domain Reassignment to avoid crashes

[Dimitry Andric via Phabricator via llvm-commits]

dim added inline comments.


================
Comment at: lib/Target/X86/X86DomainReassignment.cpp:753
 
   for (auto I : Converters)
     delete I.second;
----------------
craig.topper wrote:
> Use DeleteContainerSeconds from STLExtras.h, or better yet change Converts to hold std::unique_ptrs and then just call clear.
Sorry about that, I hadn't seen your remark earlier.  I tried using `std::unique_ptr` for `Converters`, but that doesn't work:

```
/home/dim/src/llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp:453:27: error: no viable conversion from 'std::__1::unique_ptr<(anonymous namespace)::InstrConverterBase, std::__1::default_delete<(anonymous namespace)::InstrConverterBase> >' to '(anonymous namespace)::InstrConverterBase *'
      InstrConverterBase *IC = Converters.lookup({i, MI->getOpcode()});
                          ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/dim/src/llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp:17:
In file included from /home/dim/src/llvm/trunk/lib/Target/X86/X86InstrInfo.h:17:
In file included from /home/dim/src/llvm/trunk/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:20:
In file included from /home/dim/src/llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h:17:
In file included from /home/dim/src/llvm/trunk/include/llvm/MC/MCStreamer.h:18:
/home/dim/src/llvm/trunk/include/llvm/ADT/DenseMap.h:184:14: error: call to implicitly-deleted copy constructor of 'std::__1::unique_ptr<(anonymous namespace)::InstrConverterBase, std::__1::default_delete<(anonymous namespace)::InstrConverterBase> >'
      return TheBucket->getSecond();
             ^~~~~~~~~~~~~~~~~~~~~~
/home/dim/src/llvm/trunk/lib/Target/X86/X86DomainReassignment.cpp:453:43: note: in instantiation of member function 'llvm::DenseMapBase<llvm::DenseMap<std::__1::pair<int, unsigned int>, std::__1::unique_ptr<(anonymous namespace)::InstrConverterBase, std::__1::default_delete<(anonymous namespace)::InstrConverterBase> >, llvm::DenseMapInfo<std::__1::pair<int, unsigned int> >, llvm::detail::DenseMapPair<std::__1::pair<int, unsigned int>, std::__1::unique_ptr<(anonymous namespace)::InstrConverterBase, std::__1::default_delete<(anonymous namespace)::InstrConverterBase> > > >, std::__1::pair<int, unsigned int>, std::__1::unique_ptr<(anonymous namespace)::InstrConverterBase, std::__1::default_delete<(anonymous namespace)::InstrConverterBase> >, llvm::DenseMapInfo<std::__1::pair<int, unsigned int> >, llvm::detail::DenseMapPair<std::__1::pair<int, unsigned int>, std::__1::unique_ptr<(anonymous namespace)::InstrConverterBase, std::__1::default_delete<(anonymous namespace)::InstrConverterBase> > > >::lookup' requested here
      InstrConverterBase *IC = Converters.lookup({i, MI->getOpcode()});
                                          ^
/usr/include/c++/v1/memory:2440:3: note: copy constructor is implicitly deleted because 'unique_ptr<(anonymous namespace)::InstrConverterBase, std::__1::default_delete<(anonymous namespace)::InstrConverterBase> >' has a user-declared move constructor
  unique_ptr(unique_ptr&& __u) noexcept
  ^
2 errors generated.
```

It looks like `DenseMap::lookup` attempts to copy the object when returning it, and I don't want to mess with `DenseMap` for such a minor fix as this.


Repository:
  rL LLVM

https://reviews.llvm.org/D46425