<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Jun 22, 2015 at 2:26 PM Peyton, Jonathan L <<a href="mailto:jonathan.l.peyton@intel.com">jonathan.l.peyton@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">





<div lang="EN-US" link="#0563C1" vlink="#954F72">
<div>
<p class="MsoNormal">Hello everyone, </p><p class="MsoNormal"><u></u></p>
<p class="MsoNormal">I have been working on overhauling the CMake build system for the LLVM OpenMP Runtime library.  Instead of sending 1000 small updates, I decided to do a complete refactoring/reorganizing/renaming/you-name-it-I-did-it.  **<b>All the interface
 variables are still the same (LIBOMP_OS, LIBOMP_ARCH, etc.)**</b>.  So there was no change there.</p></div></div></blockquote><div><br></div><div>FWIW, I'm excited to see this work.</div><div><br></div><div>One question, would it be OK to use Phabricator for the review? While I agree and plan on patching and looking at the patch directly, creating comments on specific parts of the change is *much* easier for me with Phabricator, so I'll be able to review it there faster.</div><div><br></div><div>Have you looked much at the libc++ and/or compiler-rt CMake build? I've suggested this a few times before, but it may provide a useful reference for some simplifying concepts.</div><div><br></div><div>Relatedly, I think it would be good to discuss this with Chris Bieneman who has been working on the compiler-rt CMake build. Specifically, I don't know what the host compiler requirements are for the runtime, but some of our runtimes require that they are built with the just-built-Clang compiler.</div><div><br></div><div>My final high level question (which probably just reflects my not knowing the current status) is what the progress is on using the lit test infrastructure? I don't want to add a bunch of comments about that here if it really needs to be dealt with separately, but I suspect one of the key issues is going to be integrating the cmake build with the lit test infrastructure and how that works.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="#0563C1" vlink="#954F72"><div><p class="MsoNormal">Here are the major changes:<u></u><u></u></p>
<p class="MsoNormal">1) Renaming internal variables/macros/functions – everything has libomp_ or LIBOMP_ prefixed to it unless maybe if it is a local variable/parameter inside a function/macro.</p></div></div></blockquote><div>So, looking at the source code layout, what is the plan around the offload tree? I find the structure here confusing.</div><div><br></div><div>We have a 'runtime' directory, and an 'offload' directory. But they're both documented as being runtime libraries... Is there any plan to have non-runtime-library code here? My strong impression was "no", but you certainly would have more context! =]</div><div><br></div><div>If this is all going to be part of a collection of runtime libraries, and the offload stuff just hasn't been fully integrated, I would like to make a suggestion (which has some relevance here, but we might want to discuss it elsewhere in more detail) that you (or someone) re-organize the directories along the lines of:</div><div><br></div><div>.../openmp/runtime/... -> .../openmp/...</div><div>.../openmp/offload/src/... -> .../openmp/src/offload/...</div><div>.../openmp/offload/doc/... -> .../openmp/doc/offload/...</div><div>.../openmp/offload/README.txt -> .../openmp/src/offload/README.txt</div><div><br></div><div>Would that make sense? As a relative newcomer to the library, it makes more sense to me,and so it might be a bit better at pulling people in, but I dunno if it just doesn't fit the larger organization.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="#0563C1" vlink="#954F72"><div><p class="MsoNormal"><u></u><u></u></p>
<p class="MsoNormal">2) config-ix.cmake added – compiler flag checks, linker flag checks, some feature checks, threading-model check.  This means cmake/${CMAKE_C_COMPILER_ID}/*Flags.cmake directories/files are deleted.<u></u><u></u></p>
<p class="MsoNormal">3) All files inside cmake/ subdirectory are renamed to Libompxyz.cmake<u></u><u></u></p>
<p class="MsoNormal">4) I cut off most of the vestigial parts that were a part of the translation of <a href="http://build.pl" target="_blank">build.pl</a> system.  There are still a few that remain like LibompExports.cmake (copying things to exports/ subdirectory)</p></div></div></blockquote><div>I think it might be worth discussing with the broader community if there is any precedent here, especially on other OSes. There might be others in the community with suggestions about how this *should* work. It's really hard for me to make suggestions because currently the comments in the BuildPLRules.cmake (haven't dug into your new patch fully yet) are in terms of ICC and compiling products that aren't in LLVM's upstream repository at all.</div><div><br></div><div>If this is solely to support building out-of-tree products around the runtime, I think you may end up needing to pull specific bits of it up into your out-of-tree code, as it doesn't make a lot of sense as is for the upstream folks.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="#0563C1" vlink="#954F72"><div><p class="MsoNormal"> and LibompMicroTests.cmake (small tests
 to run on the just created library.  This is invoked with make libomp-micro-tests), but these can easily be cut off because they are in their own files and surrounded by an if() guard in the CMakeLists.txt file.<u></u><u></u></p>
<p class="MsoNormal">5) Added LLVM-architecture detection if we are part of an LLVM build.<u></u><u></u></p>
<p class="MsoNormal">6) If you want, you can use LIBOMP_ARCH=i386 or LIBOMP_ARCH=x86_64 instead of LIBOMP_ARCH=32 or LIBOMP_ARCH=32e (although 32 and 32e still exist)<u></u><u></u></p>
<p class="MsoNormal">7) Unused functions/macros have been cut away.</p></div></div></blockquote><div>Again, thanks for working on this. I'll try to dig into the new cmake code itself next. <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="#0563C1" vlink="#954F72"><div><p class="MsoNormal"><u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">This is easier to review if you just patch and take a look at files runtime/CMakeLists.txt, runtime/src/CMakeLists.txt, cmake/Libomp*.cmake:<u></u><u></u></p>
<p class="MsoNormal">$ patch –p1 –E < CMake_overhaul_v1.patch<u></u><u></u></p>
<p class="MsoNormal">[-E removes empty files created by the patch]<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">I’ve tested this out on x86 Linux, Mac, MIC, Windows and it works, but I can’t test it on other architectures.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Leave feedback, recommendations.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">-- Johnny<u></u><u></u></p>
</div>
</div>

</blockquote></div></div>