iree icon indicating copy to clipboard operation
iree copied to clipboard

Work around circular dependency in tracing/console.c.

Open ScottTodd opened this issue 1 year ago • 1 comments

As discovered on https://github.com/iree-org/iree/pull/16454 and investigated on https://github.com/iree-org/iree/pull/17265, the 'console' provider for iree::base::tracing::provider depends on the implementation of iree_time_now() in iree/base/time.c but that build target does not depend on iree::base::base (doing so would be a circular dependency).

When linking using the system linker (not LLD) on some systems, this results in errors like

cmake -B ../iree-build-tracing/ -G Ninja . \
  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
  -DIREE_ENABLE_RUNTIME_TRACING=ON \
  -DIREE_TRACING_PROVIDER=console \
  -DIREE_BUILD_COMPILER=OFF \
  -DIREE_ENABLE_LLD=OFF
cmake --build ../iree-build-tracing --target iree-cpuinfo

[0/2] Re-checking globbed directories...
[14/14] Linking C executable tools/iree-cpuinfo
FAILED: tools/iree-cpuinfo 
: && /usr/bin/clang -O2 -g -DNDEBUG -lm tools/CMakeFiles/iree-cpuinfo.dir/iree-cpuinfo.c.o -o tools/iree-cpuinfo  runtime/src/iree/base/libiree_base_base.a  runtime/src/iree/base/internal/libiree_base_internal_cpu.a  runtime/src/iree/base/libiree_base_base.a  runtime/src/iree/base/tracing/libiree_base_tracing_provider.a && :
/usr/bin/ld: runtime/src/iree/base/tracing/libiree_base_tracing_provider.a(console.c.o): in function `iree_tracing_zone_begin_impl':
console.c:(.text+0x216): undefined reference to `iree_time_now'
/usr/bin/ld: runtime/src/iree/base/tracing/libiree_base_tracing_provider.a(console.c.o): in function `iree_tracing_zone_begin_external_impl':
console.c:(.text+0x38d): undefined reference to `iree_time_now'
/usr/bin/ld: runtime/src/iree/base/tracing/libiree_base_tracing_provider.a(console.c.o): in function `iree_tracing_zone_end':
console.c:(.text+0x3cd): undefined reference to `iree_time_now'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Possible solutions:

  • (This PR) Split iree::base::base so tracing can depend on the full 'time' library without having a circular dependency
  • Inline / copy the implementation
  • Limit the linkers allowed when using -DIREE_TRACING_PROVIDER=console (explicitly or just by remembering that there is a workaround)
  • Use another function to populate start_timestamp_ns and end_timestamp_ns
  • Reorder the link command somehow (just for these libraries/targets?)

Closes https://github.com/iree-org/iree/pull/17265

ScottTodd avatar May 23 '24 22:05 ScottTodd

not a fan of the duplication - I'd say add an iree/base/internal/time.h/.c, put the function in there as int64_t iree_time_now_ns(void), call it from the iree/base/time.c iree_time_now and also from the console tracing provider.

benvanik avatar May 23 '24 22:05 benvanik

Ping?

ScottTodd avatar May 28 '24 19:05 ScottTodd