[PATCH] D78929: [AIX][XCOFF]emit extern linkage for the llvm intrinsic symbol

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/MC/MCContext.h:105
+    /// linkage.
+    std::vector<MCSymbol *> XCOFFIntrinsicSymbols;
+    using XCOFFIntrinsicSym_iterator = std::vector<MCSymbol *>::iterator;
----------------
jasonliu wrote:
> DiggerLin wrote:
> > jasonliu wrote:
> > > Just a general comment on the data structure that's using here.
> > > Have we thought about using using a SmallPtrSet so that you don't need to worry about inserting the duplicated value into the vector?
> > /// SmallPtrSet - This class implements a set which is optimized for holding
> >  /// SmallSize or less elements.  This internally rounds up SmallSize to the next
> >  /// power of two if it is not already a power of two.  See the comments above
> >  /// SmallPtrSetImplBase for details of the algorithm.
> >  template<class PtrType, unsigned SmallSize>
> >  class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
> >    // In small mode SmallPtrSet uses linear search for the elements, so it is
> >    // not a good idea to choose this value too high. You may consider using a
> >    // DenseSet<> instead if you expect many elements in the set.
> >    static_assert(SmallSize <= 32, "SmallSize should be small");
> > 
> > There are hundreds llvm intrinsic function. I will use std::set   
> There are hundreds of llvm intrinsic functions. But are you expecting every compilation will contain hundreds of llvm intrinsic functions? I would say most of the time, you would see those less than 8intrinsic functions in one compilation unit , except for some very heavy mathematic case. 
I agree with @jasonliu, and even if `SmallPtrSet` is too small, why not use `DenseSet` like the comments say?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78929/new/

https://reviews.llvm.org/D78929