ir: If array length is uint64, should bitsize also be?

[pwaller]

Just came across 1f63577 which changes the type of array/vector lengths from int64 to uint64. This broke some code I have that multiplies by the bitsize of the element type because now they have different types.

I don't mind too much which type is used, but it seems to me that whatever logic is used to choose signed vs unsigned would apply equally well to both, and the consistency breakage is a downside

---

llvm/ir/types/types.go

Lines 208 to 214 in acfb969

	// IntType is an LLVM IR integer type.
	type IntType struct {
	// Type name; or empty if not present.
	TypeName string
	// Integer size in number of bits.
	BitSize int64
	}

---

llvm/ir/types/types.go

Lines 601 to 609 in acfb969

	// ArrayType is an LLVM IR array type.
	type ArrayType struct {
	// Type name; or empty if not present.
	TypeName string
	// Array length.
	Len uint64
	// Element type.
	ElemType Type
	}

---

[mewmew]

I cross-checked against the official LLVM implementation, and you are indeed correct. Bit size should be unsigned. So, lets make it uint64 to use the same type used in ArrayType and VectorType.

From lib/AsmParser/LLLexer.cpp:

    uint64_t NumBits = atoull(StartChar, CurPtr);
    if (NumBits < IntegerType::MIN_INT_BITS ||
        NumBits > IntegerType::MAX_INT_BITS) {
      Error("bitwidth for integer type out of range!");
      return lltok::Error;
    }

From include/llvm/IR/DerivedTypes.h:

  enum {
    MIN_INT_BITS = 1,        ///< Minimum number of bits that can be specified
    MAX_INT_BITS = (1<<24)-1 ///< Maximum number of bits that can be specified
      ///< Note that bit width is stored in the Type classes SubclassData field
      ///< which has 24 bits. This yields a maximum bit width of 16,777,215
      ///< bits.
  };

[pwaller]

It amuses me that you can have an i16777215. I wonder what breaks...

[mewmew]

Where we are at it, we should make types.AddrSpace unsigned as well.

-type AddrSpace int64
+type AddrSpace uint64

From https://github.com/llvm-mirror/llvm/blob/b2a7ed6a2614177858dfe9e7c89d76f29db03e7d/lib/AsmParser/LLParser.cpp#L1524:

bool LLParser::ParseOptionalAddrSpace(unsigned &AddrSpace, unsigned DefaultAS) {