[Openmp-commits] [PATCH] D111983: [libomptarget][DeviceRTL] Generalise and simplify cmakelists

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

I'm fairly confident that this change is correct as written because:

- The complicated part `if (LLVM_DIR)...` is copied directly from amdgpu where it was debugged into existence by other people and has built by default for a long time
- MAX_SM is unused
- The rest is carefully renaming variable and slightly reordering clauses into a sequence that seems more likely to work than the current one

The dependency description in the current file looks inconsistent, there's a top level target that seems to be unused, and I think it's optimising a bitcode library then installing the non-optimised one. That I'm not confident to change, especially at the same time as this refactor, as the odds are high that I'm missing some aspect of how cmake or the rest of the build system behave.

I mistrust cmake's documentation as it correlates poorly with observed behaviour. I also anticipate openmp breaking in a fashion I am ill equipped to repair when the cmake changes to libc++ et al land. Simplifying the current script before semantics change seems a good move.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111983