[Fix][Build System] Include necessary header files in archive distrubution

[Minterl]

closes #24590

Some header files were listed as sources and vice versa.

I took the liberty of rearranging more than just the one allocator.h, so if there's some context I should have been aware of I'd be happy to revert whatever necessary.

[egebeysel]

I took the liberty of rearranging more than just the one allocator.h, so if there's some context I should have been aware of I'd be happy to revert whatever necessary.

Thanks! I personally am not aware of any, but I'll still leave the review to @AGindinson. Could you fix the linter error and check if the release archive now includes the headers though?

[AGindinson]

Thank you for the fix! This looks good to me, bar the alphabetic ordering errors as mentioned by @egebeysel. I would suggest we get one more review.

[Manewing]

Hi :)

thank you for the fix. Unfortunately the change for the build files in IREE runtime base are not sufficient. With a dummy project of the form:

CMakeLists.txt

cmake_minimum_required(VERSION 3.21)
project(iree_dist_repro C)

set(IREE_DIST "" CACHE PATH "Path to extracted IREE distribution")

add_executable(repro main.c)

target_include_directories(repro PRIVATE
  "${IREE_DIST}/include"
)

target_link_directories(repro PRIVATE
  "${IREE_DIST}/lib"
)

target_link_libraries(repro PRIVATE
  iree_runtime_unified
)

main.c

#define IREE_ALLOCATOR_SYSTEM_CTL iree_allocator_libc_ctl
#include "iree/runtime/api.h"

int main(void) {
  iree_allocator_t allocator = iree_allocator_system();
  (void)allocator;
  return 0;
}

I still get errors during compilation on missing includes:

$ make     
[ 50%] Building C object CMakeFiles/repro.dir/main.c.o
In file included from /Users/walbroel/projects/iree/tmp_dist_build/main.c:2:
In file included from /Users/walbroel/projects/iree/build/install/include/iree/runtime/api.h:12:
/Users/walbroel/projects/iree/build/install/include/iree/hal/api.h:12:10: fatal error: 'iree/hal/allocator.h' file not found
   12 | #include "iree/hal/allocator.h"         // IWYU pragma: export
      |          ^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [CMakeFiles/repro.dir/main.c.o] Error 1
make[1]: *** [CMakeFiles/repro.dir/all] Error 2
make: *** [all] Error 2

To fix this I think you also need to update:

• runtime/src/iree/base/internal/CMakeLists.txt to expose the atomic headers in internal
• runtime/src/iree/hal/CMakeLists.txt to expose the HAL header files

[Minterl]

runtime/src/iree/base/internal/CMakeLists.txt to expose the atomic headers in internal
runtime/src/iree/hal/CMakeLists.txt to expose the HAL header files

Those files are generated by the bazel_to_cmake converter. I'll fix the bug in the bazel file when I get a chance.

[ScottTodd]

FYI, prior discussion on #20574 (cc @benvanik ).

[Minterl]

FYI, prior discussion on #20574 (cc @benvanik ).

We do not support building IREE from headers in an install. If anything, we should not be shipping any runtime headers with a default package.

As an uninformed external observer, this seems counterintuitive given the binary distributions are about two clicks away from the IREE homepage (see https://iree.dev/building-from-source/).

The compiler features a complete set of headers in the tarball, and without the context I am missing, I cannot see why the runtime should not.

I wanted to embed the IREE runtime, and the documentation lead me to believe that the distributions were the primary means of doing so (again, I may be missing something).

[ScottTodd]

The recommended way to use the runtime has been via the full source, not a binary distribution, see for example:

• https://iree.dev/reference/bindings/c-api/#samples_1
• https://github.com/iree-org/iree-template-runtime-cmake
• https://github.com/nod-ai/amd-shark-ai/blob/main/shortfin/CMakeLists.txt

This was all set up several years ago though, so if current project contributors want something different I think other choices could be made today. The archive distributions in particular have gone through very few design iterations and I'm not aware of too many users of them.

[Minterl]

My use case highly favors the binary distribution, since it would mean
a) I don't have to introduce CMake to my build toolchain (I am binding to Zig), and
b) I could have a symmetrical experience embedding the compiler and the runtime (if I was to do that) without having to build the compiler myself for development.

Obviously, development velocity in unsupported language bindings doesn't need to be a priority in decision making, but supporting binary distributions beyond their current state does make integrating IREE into external projects significantly easier.

[ScottTodd]

What are you looking for with "embedding the runtime"? IREE does not provide runtime binaries (e.g. libireeruntime.so)^ outside of a few developer tools (e.g. iree-run-module), the C/C++ runtime code is designed to be built into your application using LTO and other optimizations. You can choose to use CMake or another build system for that.

^ There were experiments with https://github.com/iree-org/iree-experimental/tree/main/runtime-library, which may be useful

[AGindinson]

@ScottTodd, thanks a lot for the explanation on the current state and all the links, it's very helpful!

I think we all agree that the runtime distribution should either be usable or get dropped altogether to avoid confusion. Given that there's clear user interest (#21571, #24590), supporting at least some of binary distribution use-cases seems like a much better direction to me. This would enable quicker experimental adoption of IREE RT in user projects, albeit with suboptimal performance, so that one can assess the functionality before deciding to build from source.

This PR is a good first step in that direction because of the aforementioned issues. I personally don't see a way around making api.h usable without all the IWYU-exported headers available. Therefore, I would still suggest to incorporate this, but we should make it clear in the docs that iree/base/api.h is the only header that should be included when using the runtime as a C dependency. Unless, of course, there are strong reasons to keep the other base/ headers non-public before some of the exposed APIs are factored out of them. What does everyone think?

However which way we improve the header situation, there's more items to handle before we claim maintained support of the dist flow – in my opinion, we'd need to add appropriate integration tests, test out if we can leverage more LTO when libiree_runtime_unified.a is used instead of from-source builds, and align the documentation so that there's clarity on supported scenarios and performance implications. I can create follow-up issues if there's alignment on the general direction.

[Manewing]

Verified this works now with the above mentioned CMake setup