tenstorrent / tt-metal

:metal: TT-NN operator library, and TT-Metalium low level kernel programming model.
https://docs.tenstorrent.com/ttnn/latest/index.html
Apache License 2.0
503 stars 83 forks source link

Refactor build so we can build arch-independent #596

Open kkwong10 opened 1 year ago

kkwong10 commented 1 year ago

Current mechanism is reliant upon ARCH_NAME specified in the build. Which is a compile-time flag that we have to use for all the different locations which we need to include for defines etc.

In general we want to move away from this in the long term or figure out a plan for this. Tagging @tt-rkim @DrJessop as well,

There are two considerations to this from a higher leve, the way to do the build should stem from this:

  1. How do we do internal maintain code-base when we have two arches
  2. How do we want to release to customers.

Since we have no plans to support heterogeneous systems, I think we can do internal testing with a compile-time flag(as it currently stands) or we can change to something more define based. But it does take more effort if things are split as they are.

For customers, we should think about this, @tt-rkim for input.

tt-rkim commented 1 year ago
kkwong10 commented 1 year ago

@tt-rkim @davorchap We want make build to not need ARCH_NAME=<>

I think if we solve 3. We can definitely make build without ARCH_NAME.

Kernels and fw are JIT compiled so we can ignore as we already solve this with a ARCH passed into compile_program

tt-rkim commented 1 year ago

@kkwong10 and I have had on off discussions concerning this.

From what I understood, the key guiding principle will be to select, at runtime, a namespaced version of the current variables which are separated by arch into different header files.

It's been a while since we discussed this issue, so @kkwong10 may have been some considerable progress already. Curious to see what he decided is a good way forward

The following modules in tt_metal will need to have ARCH_NAME torn out (just to enumerate):

TT-billteng commented 8 months ago

We should consider raising the priority of this as blackhole is coming soon

tt-rkim commented 8 months ago

cc: @jliangTT - a part of the larger BH story

tt-rkim commented 6 months ago

@TT-billteng @davorchap @rtawfik01 @pgkeller This won't be immediately contribute to features for BH or javelin, but I think this will repay us in droves once BH actually comes in.

I urge we move this up in priority.

tt-rkim commented 2 months ago

@TT-billteng @teijo

This came up in a recent weekly meeting.

There's more interest in moving this up in priority. One underlying issue is that ti's more so a UMD refactor issue

blozano-tt commented 2 months ago

Enumeration of Required Changes

afuller-TT commented 1 month ago

Interest in this from others in the community: https://discord.com/channels/863154240319258674/1177107133167321158/1293915781305864284