Further details on Type::(get|set)SubclassData?

I’m trying to grok some of the implementation details regarding MLIR Types and was wondering if someone could better explain what the “subclass data” on a type means?

  unsigned getSubclassData() const;
  void setSubclassData(unsigned val);

The TypeStorage class at least has a comment, but it is not very useful.

  /// Get the subclass data.
  unsigned getSubclassData() const { return subclassData; }

  /// Set the subclass data.
  void setSubclassData(unsigned val) { subclassData = val; }

It looks like the only in-tree use of this is for the SPIRV dialect (@antiagainst). the usage seems odd to me since it is on a custom TypeStorage subclass already and it seems like it could just have its own field for… whatever it is using it for (versus carrying it on the base TypeStorage class). But, I don’t really understand what it is using it for.

I think it’s just some default storage space that types can use without having to derive TypeStorage. I also think it’s inherited from LLVM (where it’s a 24-bit integer trying to recycle the padding due to alignment of llvm:Type) and may be either dropped entirely, or replaced by some SimpleTypeStorage that contains an unsigned.

Thanks. Wdyt about at least removing the public accessors on Type? Being able to arbitrarily mutate a uniqued type doesn’t seem like a great idea. I’m less concerned with some storage space in the implementation.

I think it makes sense, but deferring to @River707.

Do you mean setSubclassData ? Or the getter too? Seems used beyond Spirv (llvm-project/Types.h at 63a4fdda8c33174dc9fc80fed385df759abbf14f · llvm/llvm-project · GitHub) unless I’m looking at the wrong function :slight_smile:

Ah, I missed that use (grepped everything but the source files I was looking at :slight_smile:). I think we should remove Type::setSubclassData because it is dangerous. Possibly also make Type::getSubclassData protected to encourage using it only in the way that FunctionType is using it (as an implementation detail).