Remove tt::target::DataType::None #1103
Conversation
jnie-TT
requested review from
kmabeeTT,
AleksKnezevic,
pilkicTT,
sdjordjevicTT,
nsmithtt,
rpavlovicTT,
svuckovicTT,
mtopalovicTT,
mrakitaTT,
nobradovictt,
jserbedzijaTT and
tapspatel
as code owners
| @@ -18,9 +18,9 @@ table ToMemoryConfigOp { | |||
| table ToLayoutOp { | |||
| in: tt.target.TensorRef; | |||
| layout: tt.target.TensorLayout; | |||
| dtype: tt.target.DataType; | |||
| dtype: tt.target.DataType = null; | |||
There was a problem hiding this comment.
There's no need to set this to null, each field is optional in flatbuffers, and defaults to 0/null.
There was a problem hiding this comment.
For enums it has to be set, 0/null only works for flatbuffers::Offset types which tables get compiled to. Enums get directly compiled to C++ enums. That's why I previously had to add a NONE type to the enum type to make it behave similarly. If we set it to 0 it would assume we're assigning the enum value that maps to 0.
There was a problem hiding this comment.
Oh I see, thanks for letting me know!
jnie-TT
force-pushed
the
jnie/none_data_type
branch
3 times, most recently
from
bb70931 to
fb5d6c1
Compare
| @@ -35,8 +35,8 @@ void run(const ::tt::target::ttnn::ToLayoutOp *op, ProgramContext &context) { | |||
| std::optional<::ttnn::MemoryConfig> memoryConfig = std::nullopt; | |||
| ::ttnn::Device *device = nullptr; | |||
|
|
|||
| if (op->dtype() != ::tt::target::DataType::None) { | |||
| dtype = ::tt::runtime::ttnn::utils::toTTNNDataType(op->dtype()); | |||
| if (op->dtype()) { | |||
There was a problem hiding this comment.
Will this work for Float32?
There was a problem hiding this comment.
Yes it will because the enum is wrapped in a flatbuffers::Optional (because we're assigning null in the fbs file), so this check will solely check if the Optional has a value and is not a flatbuffers::nullopt. That's also why I dereference the dtype on the next line to extract the enum value.
jnie-TT
force-pushed
the
jnie/none_data_type
branch
3 times, most recently
from
d9d0cd9 to
146fc32
Compare
jnie-TT
force-pushed
the
jnie/none_data_type
branch
from
146fc32 to
3840b83
Compare
Closes #1102