delete called on non-final 'CoordinateManager' that has virtual functions but non-virtual destructor

[nsmithtt]

tt-mlir's latest uplift integration fails because of the following compile error:

/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/unique_ptr.h:85:2: error: delete called on non-final 'CoordinateManager' that has virtual functions but non-virtual destructor [-Werror,-Wdelete-non-abstract-non-virtual-dtor]
   85 |         delete __ptr;
      |         ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/unique_ptr.h:361:4: note: in instantiation of member function 'std::default_delete<CoordinateManager>::operator()' requested here
  361 |           get_deleter()(std::move(__ptr));
      |           ^
/__w/tt-mlir/tt-mlir/third_party/tt-metal/src/tt-metal/tt_metal/third_party/umd/device/tt_soc_descriptor.h:140:5: note: in instantiation of member function 'std::unique_ptr<CoordinateManager>::~unique_ptr' requested here
  140 |     tt_SocDescriptor(const tt_SocDescriptor& other) :

The fix should be to define a virtual destructor for class CoordinateManager.

[nsmithtt]

fyi @pjanevskiTT

[pjanevskiTT]

Looking at it

[pjanevskiTT]

https://github.com/tenstorrent/tt-mlir/tree/pjanevski/fix_metal_uplift_build @nsmithtt let me know if the problem goes away. For me localy it does

[nsmithtt]

@pjanevskiTT, what is the uplift strategy when uplifting UMD into tt-metal? We'd rather take vanilla main if we think tt-metal can adopt your fix in main.

[pjanevskiTT]

Well we try to open the PR for every UMD change that requires API changes inside tt-metal, but merging that can sometimes take some time. Problems can occur with something like this that does not need to be uplifted in tt-metal, but you need the change. Do you have some suggestion on how we should do this?

[nsmithtt]

Presumably UMD recently uplifted (within the past couple weeks) because we just started seeing this error. Would it be possible to uplift again now? Who can I contact regarding this process?

I think in this case we'll just be leaking memory.

[pjanevskiTT]

I can uplift it. Let me create a branch with this commit included and run post-commit

[pjanevskiTT]

As soon as post-commit tests pass https://github.com/tenstorrent/tt-metal/actions/runs/11687357791, I will open a PR, if that sounds ok

[nsmithtt]

That's ok with me, fyi @blozano-tt @afuller-TT @vtangTT @broskoTT

[pjanevskiTT]

Closing the issue because it is fixed within umd, @nsmithtt I will tag all the people above in the tt-metal uplift