"cannot have signedness semantics for i1"?

cc @antiagainst

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 sext and 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.

-Chris

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.

– River

+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.

1 Like

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 BoolAttr side. :slight_smile:

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!

-Chris

Thank you for the help, I landed the patch in 1c0efa8b547