Quant dialect: issue with the storage type

Hello,

I’m trying to use the quant::UniformQuantizedType type, but I’m a bit confused with the storage type.

If I give a signed integer as storage type, the type is dumped as ‘i8’, which means that when reloading the MLIR file, I’ll get a signless integer 8 bits.

Then, my question is : is the storage type of quantized type expected to be signless ?

Thanks

The quant dialect predated signed/unsigned types in MLIR and follows the convention of using signless types. The type maintains its sign flag as a separate bit: https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Quant/QuantTypes.h#L105

The printer/parser will mangle the storage type to represent unsigned based on that flag: https://github.com/llvm/llvm-project/blob/9e3678e161553f29df1408f1b7c7ffc14fc0ef17/mlir/test/Dialect/Quant/parse-uniform.mlir#L25

Thanks for your clear and fast answer.

The current situation is IMO possibly confusing for newcomers.
On my side, I was confused 2 times:

  • when creating the UniformQuantizedType, I was surprised to have to provide the signess information while I had already given it in the storage type
  • when dumping and reloading the IR, I was surprised to get a different storage type

I see two possible evolutions that may help preventing this type of confusion:

  1. we could forbid signed or unsigned integers as storage type.
  2. we could let the storage type carry the sign information (forbidding signless storage types) and remove the redundant sign flag passed to the quant type.

Personally, I have a preference for (2) since the storage class would directly correspond to the actual computation type.

Just so I understand, you are constructing a UniformQuantizedType with a signed integer type (i.e. si8) not a signless (i.e. i8)? As mentioned, the entire dialect predated the introduction of signed/unsigned types and follows the typical LLVM convention of using signless integers and maintaining the signedness separately where it is important. Given the current state of it, the right thing would likely be to require that the UQT can only be constructed with signless integers. That restriction is not implemented because they didn’t exist when it was created.

This could certainly be made more ergonomic. At this point, I would likely opt to not change the internal representation of the type (i.e. it operates on signless integers with an unsigned flag) but we could change the builder and accessors to let you use signed/unsigned types and bridge those properly. I’d rather not change the internal representation or the printed form at this point as it has fairly wide use, but we can make it better for interop with newer types.

Yes, an option is indeed to extend the API to make it work with signed/unsigned integers (builder and also getters since requesting the storage type would always return a signless integer). Keeping backward compatibility by keeping the same internals and the same behavior of the existing API looks good to me.

Something I’ve noticed also is the usage of ‘i8’ (for signed) and ‘ui8’ (for unsigned) in the dump. ‘i8’ is usually for signless integers so I think it would be more consistent to use ‘si8’ instead.

Yeah, like I mentioned, this follows the LLVM signless convention and the printer predates MLIR’s signed/unsigned types and was chosen arbitrarily. It made sense at the time.

I’m somewhat -1 on cosmetic changes to the printed form without a strong reason to do so.

I think that “alignment of the syntax on the rest of MLIR” is more than cosmetic: it is quite confusing that i8 isn’t parsing as MLIR i8!

FTR - it is parsing as an MLIR i8 and the type carries a flag indicating whether these should be interpreted as unsigned quantities in contexts where that is important (it mostly isn’t). This is pretty aligned with how LLVM reasons about signless integers. For convenience, when the type was created, the printer prints (i8, unsigned_bit==1) as u8 but could have also spelled this as i8 unsigned or something more explicit.

I think that “preference for signless integer conventions” in LLVM are often a source of confusion for folks coming from outside LLVM and isn’t itself a reason to go the other way.

If we’re changing the way it prints, I’d rather align it closer to the internal representation (which uses a signless type) rather than the cute i8u8 string munging it does now. I agree that is confusing.

But, consider this a strong opinion, weakly held.

Ah all good then: I misunderstood the point above!