[PATCH] D107034: [CSPGO] Set EnableValueProfiling flag for CSPGO to avoid IRPGO value loss in LTO

[Yolanda via Phabricator via llvm-commits]

YolandaCY marked an inline comment as done.
YolandaCY added a comment.

In D107034#2945640 <https://reviews.llvm.org/D107034#2945640>, @MaskRay wrote:

> Can you give concrete instructions to reproduce the linker error?

I tried to create a small reproducer but still encountered several issues. The linker error may be explained with below example:

  class Str {
    int size;
  
  public:
    char *p;
  
    Str(int size) {
      p = new char[size];
    }
  
    ~Str() {
      delete[] p;
    }
  
    void assign_mem(char *a, char *b, int size) {
      memcpy(a, b, size);
    }
  
    int assign(Str &b) {
      if (b.size < size) return -1;
      assign_mem(p, b.p, size);
      return 1;
    }
  };

If function `assign` inlines `assign_mem`, it does contain mem op and should require a value profile var, but not vice versa.
The case encounterd the linker error is that a function similar to `assign` (e.g. `basic_string::operator=`) does not inline the callee `assign_mem` in the splitted regular LTO link, thus it will generate a `__profd_assign*` with `private` linkage, while in a later ThinLTO module, the `assign` inlined the callee containing mem op, thus it may generate a `__profd_assign` with `non-private` linkage (due to NS!=0). During the final link together with both regular and thin LTO modules for COFF, symbols from regular LTO are inserted earlier as prevailing, and the private  `__profd_assign*` is not inserted. Whereas the symbol from thin module has external attribute, and is associated to `__profc_assign*` who is non-prevailing and should not be inserted. While there is a prevailing `__profc_assign*` existing in symbol table, there is no `__profd_assign*`, thus reported as unresolved symbol.

I think by correcting the profile var name with a hash suffix when IRPGO enabled should avoid such inconsistency in different translation units. And the linker error helped exposing the issue.

I'm implementing a CL <https://chromium-review.googlesource.com/c/chromium/src/+/2823516> to build Chromium that depends on this. The steps described in the CL should help reproduce the error if you are interested @MaskRay. The original error looks like following:

  lld-link: error: undefined symbol: __profd_??4?$basic_string at DU?$char_traits at D@__1 at std@@V?$allocator at D@23@@__1 at std@@QEAAAEAV012 at AEBV012@@Z
  >>> referenced by .\..\..\buildtools\third_party\libc++\trunk\include\string:2401
  >>>               ./chrome.dll.lto.147.obj:(public: class std::__1::basic_string<char, struct std::__1::char_traits<char>, class std::__1::allocator<char>> & __cdecl std::td::__1::allocator<char>>::operator=(class std::__1::basic_string<char, struct std::__1::char_traits<char>, class std::__1::allocator<char>> const &))
  >>> referenced by .\..\..\buildtools\third_party\libc++\trunk\include\string:2401
  >>>               ./chrome.dll.lto.147.obj:(public: class std::__1::basic_string<char, struct std::__1::char_traits<char>, class std::__1::allocator<char>> & __cdecl std::td::__1::allocator<char>>::operator=(class std::__1::basic_string<char, struct std::__1::char_traits<char>, class std::__1::allocator<char>> const &))

Please let me know if more information is needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107034