[PATCH] D96455: [SampleFDO][NFC] Refactor SampleProfile.cpp

[Rong Xu via llvm-commits]

Adding "inline" keywords will make the functions COMDAT and will fix the
duplicates problem. I verified with -DLLVM_ENABLE_MODULES=On.

Once we make the class a template, we don't need the "inline" keywords. I
also verified this with my template patch.

I will do a few more tests and will commit a modified patch with "inline"
keywords.

On Wed, Feb 17, 2021 at 11:57 AM Vedant Kumar via Phabricator <
reviews at reviews.llvm.org> wrote:

> vsk added a comment.
>
> This particular bot is using -DLLVM_ENABLE_MODULES=On. I'm not sure how to
> explain why the issue doesn't pop up on other bots. My current theory is
> that the LLVM_ProfileData module re-exports the new header because of:
>
>   module LLVM_ProfileData {
>     requires cplusplus
>
>     umbrella "ProfileData"
>     module * { export * } //< This?
>
>     textual header "ProfileData/InstrProfData.inc"
>   }
>
> If SampleProfileLoaderBaseImpl is indeed imported from Transforms/Utils,
> then the re-export from the ProfileData module might cause the duplicate
> definition link failure, even though there's only one #include for the
> header.
>
> I'd wager that the simplest fix would be to pull the header definitions
> from SampleProfileLoaderBaseImpl into a .cpp. If/when those definitions
> become templated they can move back into the header. Alternatively perhaps
> the modulemap file can be changed to not re-export
> SampleProfileLoaderBaseImpl, but I don't know how to make that change or
> how invasive it'd be.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D96455/new/
>
> https://reviews.llvm.org/D96455
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210217/8fe4ef12/attachment.html>