[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/Headers/CMakeLists.txt:421
 
 install(
   FILES ${ppc_wrapper_files}
----------------
qiongsiwu1 wrote:
> craig.topper wrote:
> > qiongsiwu1 wrote:
> > > craig.topper wrote:
> > > > There appear to be two installs of ppc_wrapper_files with different components. Is that intentional?
> > > Ah yes this is indeed intentional. `cuda_wrapper_files` and `openmp_wrapper_files` have two install targets as well. The first target is part of the "catch-all" `clang-resource-headers`.  The second targets are not installed by default (`EXCLUDE_FROM_ALL`). and a distribution build can opt-in the relevant set of headers. 
> > > 
> > > That said, if there are better ways to implement the logic I am all ears. 
> > As far as I know, the ppc_wrapper_files are only usable on the powerpc target so I don't think they should be treated any different than ppc_files. ppc_files only has one install target right?
> > As far as I know, the ppc_wrapper_files are only usable on the powerpc target 
> Yes that is my understanding as well. 
> 
> > ppc_files only has one install target right?
> There is indeed one install target specifically setup to include only ppc files. Meanwhile, the ppc files are also included in the `${files}` list (which is installed as as a component of the catch-all `clang-resource-headers`), so the ppc files are included in two different install targets. 
> 
Thanks. I was trying to resolve some merge issues in my downstream and I really should have read your changes more closely.


================
Comment at: clang/lib/Headers/CMakeLists.txt:548
+install(
+  FILES ${windows_files}
+  DESTINATION ${header_install_dir}
----------------
Should this be windows_only_files. I don't see a windows_files defined anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498