[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

Tom Tan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 11 12:35:07 PST 2019


TomTan added a comment.

In D57915#1393510 <https://reviews.llvm.org/D57915#1393510>, @thakis wrote:

> In D57915#1389722 <https://reviews.llvm.org/D57915#1389722>, @TomTan wrote:
>
> > In D57915#1389560 <https://reviews.llvm.org/D57915#1389560>, @lebedev.ri wrote:
> >
> > > In D57915#1389549 <https://reviews.llvm.org/D57915#1389549>, @TomTan wrote:
> > >
> > > > Added the tests back. Clang IR should not lower these to bswap calls because they are global library functions. It might be slower to make the call to library function than bswap, but this is the same for other architectures supported by Windows. And just redefine global library function triggers link error in some scenarios.
> > >
> > >
> > > I think there is some deeper reasoning is being omitted here.
> > >  //Why// does the fact that those are "global library functions" bans clang from lowering them into IR?
> > >  (and thus, "prevents" LLVM form doing optimizations)
> >
> >
> > The current issue could be caused by the implementation of comdat selection in LLD as below which provides more strict conflict check than MSVC link does.
>
>
> lld isn't supposed to be more strict than link.exe. lld used to not implement the comdat selection field until recently, so lld got more strict – but it should've gotten only as strict as link.exe, not moreso. Do you have an example where lld is more strict than link.exe?


My previous reply is LLD comdat linking issue was misleading. The current issue is that when function declaration (`extern`) and definition (`static inline`) shown up in sequence, the function declaration property `extern` wins over `static` in the resulting COMDAT, which causes linking issue for both LLD and link.exe. But this issue was just exposed by the recent addition of duplicated symbols check in LLD.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57915





More information about the cfe-commits mailing list