IntegerType::verifyConstructionInvariants in lib/IR/StandardTypes.cpp has the following check:
if (width == 1 && signedness != IntegerType::Signless)
return emitOptionalError(loc, "cannot have signedness semantics for i1");
This prevents making signed and unsigned single bit types. These types are quite well defined and meaningful - LLVM and MLIR support
zext from the signless
i1 types afterall.
Would anyone oppose removing this check? It seems like it was intentionally thought about, but I don’t understand the rationale for the restriction.
I probably requested the restriction, but I don’t remember what rationale I had in mind and it probably didn’t make sense. I’m +1 for removing it. The only problem I foresee is with BoolAttr, which would now need to accept different i1 types? That is a somewhat easy problem to solve though, as I have intended to change BoolAttr to be a wrapper around I1 IntegerAttr for a while now.
+1 for removing it. There’s no reason it’s not well-defined.
I don’t think there is any need to make the “true” and “false” tokens be allowed to be signed. Is there some other case you’re concerned about?
To be clear, I’d like to allow things like:
1 : ui1 and
0 : ui1
I’m not suggesting that we allow
true : ui1 etc.
IIRC, the main reason I put that check is because of
i1's dual purpose: it’s used both for 1-bit integer type and bool type. As River pointed out, we have a dedicated
BoolAttr different from
IntegerAttr but there is no
BoolType separate from
IntegerType. Also I find it a bit hard to understand a signed
i1. Does the whole value just become a sign?
a 1 bit signed type can express 2 values: -1 and 0.
Makes sense. Cool. SGTM to remove. I guess River will sort out the
Thanks. I’ll send a patch for review when I get a minute, perhaps later today.
I agree with you. The current situation is that some things, AFAIR, use BoolAttr as opposed to IntegerAttr I1(e.g. DenseElementsAttr). What I would like is to make BoolAttr just be a sugared wrapper around signless I1 integer attributes. That way we avoid this awkward dance around how to represent I1.
Cool thanks. I wasn’t aware of how
BoolAttr worked. I posted the patch in question here. Thank you for the help!
Thank you for the help, I landed the patch in 1c0efa8b547