"cannot have signedness semantics for i1"?

MLIR
#1

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

#2

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

#3

+1 for removing it. There’s no reason it’s not well-defined.

#4

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?

#5

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
#6

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?

#7

a 1 bit signed type can express 2 values: -1 and 0.

#8

Makes sense. Cool. SGTM to remove. I guess River will sort out the BoolAttr side. :slight_smile:

#9

Thanks. I’ll send a patch for review when I get a minute, perhaps later today.

#10

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.

#11

Cool thanks. I wasn’t aware of how BoolAttr worked. I posted the patch in question here. Thank you for the help!

-Chris

#12

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