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
485 stars 79 forks source link

DST_ACCUM_MODE not forwarded for some functions with default parameters. #14106

Open amahmudTT opened 1 month ago

amahmudTT commented 1 month ago

Some functions have default argument for the template parameter defining is_fp32_dest_acc_en. When we generated the define DST_ACCUM_MODE which was meant to be passed to all the functions taking in the template parameter is_fp32_dest_acc_en. We missed some of those which have default arguments for this parameter. This may be harmless in some cases but opens up opportunities for bugs.

One example is

template <bool is_fp32_dest_acc_en = false>
inline void llk_pack_dest_section_done() {
    _llk_pack_dest_section_done_<DST_SYNC_MODE, is_fp32_dest_acc_en>();
}

We call the above function from /tt_metal/include/compute_kernel_api/reg_api.h

ALWI void tile_regs_release() {
    PACK(( llk_pack_dest_section_done()  ));
}

We need to remove the default arguments and pass the defined DST_ACCUM_MODE that is defined. In the above example having the right code would actually follow a different code path.

amahmudTT commented 1 month ago

@rdjogoTT FYI

amahmudTT commented 3 weeks ago

This turned out to be a big change, fixing this in one single PR is impossible as many files have the default argument in the middle and its position needs to be changed if other default arguments are to be maintained. The call hierarchy needs to be correct too. since sometimes the template argument is passed down the chain, so instead of trying to make one large PR, we need to handle one low level/third party file along with its consumers at a time. Say one file in the third party should be modified while fixing everything related to that file in the call chain.

We could use this PR as one of the cleanup items to help new-hires get used to the pipeline. I mean submitting PRs, reviewing them etc.