compnerd added inline comments.
================
Comment at: clang/cmake/modules/AddClang.cmake:127
+ LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+ ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+ RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})
----------------
Ericson2314 wrote:
> compnerd wrote:
> > Ericson2314 wrote:
> > > compnerd wrote:
> > > > For the initial change, Id leave this off. `CMAKE_INSTALL_LIBDIR`
> > > > sometimes already deals with the bit suffix, so you can end up with two
> > > > instances of `32` or `64`. I think that cleaning that up separately,
> > > > while focusing on the details of cleaning up how to handle
> > > > `LLVM_LIBDIR_SUFFIX` is the right thing to do. The same applies to the
> > > > rest of the patch.
> > > https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/GNUInstallDirs.cmake#L257
> > > Hmm I see what you mean. So you are saying `s/${
> > > CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}/${ CMAKE_INSTALL_LIBDIR}/`
> > > everywhere?
> > Yes, that is what I was referring to. I'm suggesting that you do *not*
> > make that change instead. That needs a much more involved change to clean
> > up the use of `${LLVM_LIBDIR_SUFFIX}`. I think that this change is already
> > extremely large as is, and folding more into it is not going to help.
> So you are saying II should back all of these out into
> `lib${LLVM_LIBDIR_SUFFIX}` as they were before, for now?
>
> Yes I don't want to make this bigger either, and would rather be on the hook
> for follow-up work than have this one be too massive to get over the finish
> line.
Yes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99484/new/
https://reviews.llvm.org/D99484
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
