[PATCH] D25965: Utility functions for appending to llvm.used/llvm.compiler.used

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: lib/LTO/UpdateCompilerUsed.cpp:117
                               const StringSet<> &AsmUndefinedRefs) {
-  SmallPtrSet<const GlobalValue *, 8> UsedValues;
   PreserveLibCallsAndAsmUsed(AsmUndefinedRefs, TM, UsedValues)
----------------
eugenis wrote:
> mehdi_amini wrote:
> > This is a set, which ensure no duplicate.
> Now I may be missing something, but this set is converted to a vector before the current llvm.compiler.used is read. There is now deduplication between the current entries of llvm.compiler.used and the new elements. And, as Peter explained above, the new elements are unique among themselves.
You're right. However it is a bug I introduced when refactoring this in http://reviews.llvm.org/D19000 ; the original code was turned the set into a vector only after appending the existing elements. So we should fix this bug and not simplify the code assuming the buggy behavior is expected. 
I don't expect this API, `updateCompilerUsed`, to introduce a duplicate here.


Repository:
  rL LLVM

https://reviews.llvm.org/D25965