oneDNN icon indicating copy to clipboard operation
oneDNN copied to clipboard

[graph cmake] cmake utils in graph-dev vs master neither disjoint nor the same

Open robogast opened this issue 2 years ago • 1 comments

Summary

By default, building oneDNN graph from source means also building oneDNN (original) from source, through build_oneDNN.cmake. Both graph and original contain a cmake/utils.cmake, which share a significant overlap in functions and macros, but this overlap is both different in functionality and is missing at least one key macro (set_if, without this macro OpenMP.cmake is invalid).

register_exe and maybe_configure_windows_test are redefined in-place in graph (meaning functionality is changed), whereas for example set_ternary, set_if, (and many others,) are missing from graph-dev.

Issues

This non-disjointness / non-sameness creates some issues:

  1. Behaviour of the functions/macros is dependent on order of inclusion in the build
  2. Building oneDNN on its own and then linking it to oneDNN is not possible, because the macro set_if is missing.

Options

In my view, there are two distinct options:

  1. Either: building oneDNN independently is explicitely disallowed. which means the graph-dev utils.cmake file can only contain disjoint functions and macros from original utils.cmake, otherwise you run into issue 1.
  2. Or: building oneDNN independently is allowed, meaning oneDNN-graph should be self-contained (as far as used macros and functions), thus macros such as set_if are included in its own utils.cmake, and macros and functions from graph-dev must be carbon copies of oneDNN original macros and functions, otherwise you run into issue 1 again.

My personal preference is option 2, because I would like to be in control of the build flags of my software (I manage part of the software stack of a HPC system). If you say I must build oneDNN (original) at the same time I build oneDNN graph, then please at least make sure the conditions of option 1 hold.

Example

To clarify what I mean, I include the following snippet from vimdiff (left = master, right = graph-dev), in which you can see that (in order from top to bottom):

  1. a functions behaviour is overwritten
  2. a function is changed and has a different name
  3. a functions behaviour is overwritten
  4. a macro is duplicated
  5. a macro is excluded

image

robogast avatar Jul 07 '22 11:07 robogast

Thank you for the report, @robogast. The build structure and API in tech previews is indeed a bit disjoined and we are working on more uniform look and feel for the Graph component in next releases.

@TaoLv, @igorsafo

vpirogov avatar Jul 07 '22 14:07 vpirogov

@robogast, oneDNN Graph Beta release has Graph component fully integrated into oneDNN. This should fix the issues you see.

vpirogov avatar Oct 14 '22 19:10 vpirogov