Add support for zero-sized Scalars as a LowLevelType

[pmatos]

Summary

Opaque values (of zero size) can be stored in memory with the implemention of
reference types in the WebAssembly backend. Since MachineMemOperand
uses LLTs we need to be able to support zero-sized scalars types in LLTs.

[pmatos]

Adding @arsenm as reviewer since he committed https://github.com/llvm/llvm-project/commit/990278d026d680942c859be70836ad34a9a716f7 , which broke d104797 and forced this patch to be required.

[pmatos]

Adding @sdesmalen as reviewer, who was one of the last authors of changes to LLT implementation.

[pmatos]

Apply clang-format changes

[arsenm]

I'm not sure what an opaque type means. Do you have an IR sample?

[pmatos]

In D105423#2861795, @arsenm wrote:

I'm not sure what an opaque type means. Do you have an IR sample?

Sure - for example, from how we represent externrefs (opaque object references in WebAssembly):

%extern = type opaque
%externref = type %extern addrspace(1)* ;; addrspace 1 is nonintegral
@externref_global = local_unnamed_addr addrspace(2) global %externref undef

define %externref @return_externref_global() {
  ;; this generates a global.get of @externref_global
  %ref = load %externref, %externref addrspace(2)* @externref_global
  ret %externref %ref
}

[arsenm]

In D105423#2862010, @pmatos wrote:

In D105423#2861795, @arsenm wrote:

I'm not sure what an opaque type means. Do you have an IR sample?

Sure - for example, from how we represent externrefs (opaque object references in WebAssembly):

%extern = type opaque
%externref = type %extern addrspace(1)* ;; addrspace 1 is nonintegral
@externref_global = local_unnamed_addr addrspace(2) global %externref undef

define %externref @return_externref_global() {
  ;; this generates a global.get of @externref_global
  %ref = load %externref, %externref addrspace(2)* @externref_global
  ret %externref %ref
}

But here it's just a pointer with an address space and you aren't actually using the opaque type as a value. Why can't you just treat this as a regular pointer?

[tlively]

In D105423#2862011, @arsenm wrote:

In D105423#2862010, @pmatos wrote:

In D105423#2861795, @arsenm wrote:

I'm not sure what an opaque type means. Do you have an IR sample?

Sure - for example, from how we represent externrefs (opaque object references in WebAssembly):

%extern = type opaque
%externref = type %extern addrspace(1)* ;; addrspace 1 is nonintegral
@externref_global = local_unnamed_addr addrspace(2) global %externref undef

define %externref @return_externref_global() {
  ;; this generates a global.get of @externref_global
  %ref = load %externref, %externref addrspace(2)* @externref_global
  ret %externref %ref
}

But here it's just a pointer with an address space and you aren't actually using the opaque type as a value. Why can't you just treat this as a regular pointer?

IIUC, the "opaque" type (in the sense of this patch) here is %externref, which lowers to a zero-sized (i.e. no bit representation) MVT. So it is a regular pointer except that it has zero size, which requires special care here and elsewhere in https://reviews.llvm.org/D104797. From @pmatos's latest comments on that other revision, perhaps an alternative to this patch would be to allow scalar LLTs to have zero size?

llvm/include/llvm/Support/LowLevelTypeImpl.h

273–274

Why did the number of elements change? What is the third element for? It would be good to update the comment.

277

How are the bitfields packed for opaque types?

llvm/lib/CodeGen/MachineOperand.cpp

1055 ↗

(On Diff #356931)

If this was the problematic piece of code, I'm surprised LLT::pointer isn't used anywhere here. My model for opaque as used here is that it is a zero-sized pointer rather than a zero-sized scalar. Am I thinking about it incorrectly?

[pmatos]

In D105423#2862135, @tlively wrote:

In D105423#2862011, @arsenm wrote:

In D105423#2862010, @pmatos wrote:

In D105423#2861795, @arsenm wrote:

I'm not sure what an opaque type means. Do you have an IR sample?

Sure - for example, from how we represent externrefs (opaque object references in WebAssembly):

%extern = type opaque
%externref = type %extern addrspace(1)* ;; addrspace 1 is nonintegral
@externref_global = local_unnamed_addr addrspace(2) global %externref undef

define %externref @return_externref_global() {
  ;; this generates a global.get of @externref_global
  %ref = load %externref, %externref addrspace(2)* @externref_global
  ret %externref %ref
}

But here it's just a pointer with an address space and you aren't actually using the opaque type as a value. Why can't you just treat this as a regular pointer?

IIUC, the "opaque" type (in the sense of this patch) here is %externref, which lowers to a zero-sized (i.e. no bit representation) MVT. So it is a regular pointer except that it has zero size, which requires special care here and elsewhere in https://reviews.llvm.org/D104797. From @pmatos's latest comments on that other revision, perhaps an alternative to this patch would be to allow scalar LLTs to have zero size?

Sorry @arsenm , I should have explained it better, as @tlively did instead of posting the example and leaving for the day.

In any case, as @tlively said the MVT for externref is zero-sized. See https://github.com/llvm/llvm-project/blob/0fd5e7b2d8ca4ed46d76187feb4b903ed0a3ea75/llvm/include/llvm/Support/MachineValueType.h#L1049

@tlively actually your suggestion of making the opaque type a scalar type of size zero was my first attempt : Failed! Why? The idea what that I would differentiate between a valid scalar and an opaque type depending on if its value was zero. So, I actually implemented an initial patch where I had something like this:

bool IsOpaque() { return isValid() && !IsPointer && !IsVector && getSizeInBits() == 0; }
bool IsScalar() { return isValid() && !IsPointer && !IsVector && getSizeInBits() != 0; }

I also created a constructor to construct an opaque type as a scalar type of size zero. But this didn't work because the implementation of getSizeInBits() itself calls IsScalar(). So this resulted in an infinite recursive call. I then decided to create its own type for Opaque as in this patch.

However, I am looking at the code again and notice that actually, I could call in IsOpaque() the function getScalarSizeInBits() instead of getSizeInBits(). That could work. I will get this patch redone, test, and submit it as an alternative revision.

[pmatos]

llvm/lib/CodeGen/MachineOperand.cpp

1055 ↗

(On Diff #356931)

To be honest I was quite surprise as well when looked at it. I initially thought LLT::scalar(8 * s) should be a pointer instead. However, @arsenm added a call to the scalar constructor instead, and I followed the lead for opaque. My understanding here atm for the use of LLT::scalar for pointers is that this is the type representation of a pointer in memory - a scalar of a certain size, and not the representation of the value pointed to.

[pmatos]

llvm/include/llvm/Support/LowLevelTypeImpl.h

273–274

I misunderstood the point of this array. Reverting the change. Thanks.

[pmatos]

Change representation of opaque types as scalars of size zero

[pmatos]

Remove extraneous code

[pmatos]

Remove conditional on zero size

[pmatos]

OK - I feel like this became an easier diff to deal with by doing opaques as scalar of size 0. However, to be honest I am not totally convinced yet it's useful to introduce the idea of "opaque" here. Just like we create MVT's that are Fixed(0) maybe we should just allow LLT's to have scalar(0) calls and remove this "opaque" concept here. What do others think?

[arsenm]

In D105423#2864145, @pmatos wrote:

OK - I feel like this became an easier diff to deal with by doing opaques as scalar of size 0. However, to be honest I am not totally convinced yet it's useful to introduce the idea of "opaque" here. Just like we create MVT's that are Fixed(0) maybe we should just allow LLT's to have scalar(0) calls and remove this "opaque" concept here. What do others think?

That would be a bit nicer than taking a bit just for this. However some places are relying on invalid LLTs that are all zeroes currently

[tlively]

Nice! This is a lot simpler. I agree that it would be nicer to avoid introducing a new opaque concept here entirely, but if fixing up users that depend on zero-sized LLTs to be invalid looks tricky, then this solution seems reasonable.

[pmatos]

In D105423#2864645, @tlively wrote:

Nice! This is a lot simpler. I agree that it would be nicer to avoid introducing a new opaque concept here entirely, but if fixing up users that depend on zero-sized LLTs to be invalid looks tricky, then this solution seems reasonable.

My idea what to just remove the idea of opaque. I think that a scalar 0 is different from

In D105423#2864645, @tlively wrote:

Nice! This is a lot simpler. I agree that it would be nicer to avoid introducing a new opaque concept here entirely, but if fixing up users that depend on zero-sized LLTs to be invalid looks tricky, then this solution seems reasonable.

Actually currently the solution is marking RawData as 0 for opaque types without any further special handling so it's actually invalid for those calling LLT::isValid().
I can remove the concept of opaque and allow 0 sized scalars, but will need a bit for IsScalar. Will give that a go.

[pmatos]

An update that this is taking a while because allowing the representation of zero-sized scalars breaks quite a few things. Fixing these have been a slow process, but I hope to have a proposal soon.

[pmatos]

Remove reference to opaque type and allow scalars of zero size.

As a consequence, not all scalar LLTs can now be converted to IR Integer Types.

[pmatos]

No changes to MachineOperand are necessary, reformatting to remove diff.

[pmatos]

@arsenm @tlively what does this look like to you now?

[pmatos]

Apply clang-format changes

[tlively]

Looks good beyond these clarification questions! It would also be good to update the revision title and description.

llvm/include/llvm/Support/LowLevelTypeImpl.h

117

Are pointers or vectors with RawData == 0 invalid as well?

120

Why isn't it sufficient to do return IsScalar? If isScalar() and IsScalar have different meanings, could that be clarified in the names?

[arsenm]

llvm/include/llvm/Support/LowLevelTypeImpl.h

325

Leftover change?

llvm/unittests/CodeGen/LowLevelTypeTest.cpp

24

I think we're missing an EXPECT_FALSE(LLT().isValid()) test

49

What happens for a vector of opaque?

54

What happens for a vector of opaque?

[pmatos]

llvm/include/llvm/Support/LowLevelTypeImpl.h

117

Yes, before RawData == 0 meant invalid. However, now given we allow RawData to be zero for scalars, we added a bit IsScalar to mark scalars and for those allow RawData to be zero. So, to be valid, you're either a scalar (IsScalar == 1) or your raw data is nonzero (RawData != 0).

120

It's true that you could just do isValid() && IsScalar, however the other checks are extra to ensure the type was correctly built. Since you could in principle create an LLT where all the bits are 1 (IsScalar && IsPointer && IsVector), this check ensures that a true scalar doesn't have those. On the other hand, this check probably belongs elsewhere. Maybe in init... I will change this.

325

No, the RawData uses 1 less bit, so 61 instead of 62 because we are taking a bit to encode if a value is a scalar. We need this explicit bit because before a scalar was something with !IsPointer && !IsVector. However, by allowing RawData to be zero in scalar, we need a bit for scalars to verify that a value is a scalar and allow its rawdata to be zero.

llvm/unittests/CodeGen/LowLevelTypeTest.cpp

24

Where do you expect this?

54

Good point, I hadn't thought of those. I will add a test.

[pmatos]

llvm/unittests/CodeGen/LowLevelTypeTest.cpp

24

Ah, is this what you mean:

TEST(LowLevelTypeTest, Invalid) {
  const LLT Ty;

  ASSERT_FALSE(Ty.isValid());

Line 291 of this file.

[pmatos]

Update revision title and summary. Allow zero-sized scalars in vectors.

[pmatos]

llvm/include/llvm/Support/LowLevelTypeImpl.h

325

I am starting to think we could actually get away without a IsScalar, and allow a !IsPointer && !IsVector of zero size. There's a reason I added an IsScalar but I forget exactly why. I will do some further investigation.

[pmatos]

llvm/include/llvm/Support/LowLevelTypeImpl.h

325

Argh, no. Of course we can't. Because the invalid LLT is represented by all zeros, if we allow scalar to be a zero, then we need to have a bit for scalars so that the zero scalar is not marked incorrectly as invalid.

[pmatos]

Tidy up isScalar() and improve code formatting

[pmatos]

@tlively @arsenm I hope to have addressed all the pending issues.

[tlively]

LGTM with that final simplification.

llvm/include/llvm/Support/LowLevelTypeImpl.h

120

Now this can be simplified to return IsScalar because isValid() is true if IsScalar is true.

llvm/include/llvm/Support/LowLevelTypeImpl.h

 class DataLayout;
 class Type;
 class raw_ostream;
 
 class LLT {
 public:
   /// Get a low-level scalar or aggregate "bag of bits".
   static LLT scalar(unsigned SizeInBits) {
-    assert(SizeInBits > 0 && "invalid scalar size");
-    return LLT{/*isPointer=*/false, /*isVector=*/false,
+    return LLT{/*isPointer=*/false, /*isVector=*/false, /*isScalar=*/true,
                ElementCount::getFixed(0), SizeInBits,
                /*AddressSpace=*/0};
   }
 
   /// Get a low-level pointer in the given address space.
   static LLT pointer(unsigned AddressSpace, unsigned SizeInBits) {
     assert(SizeInBits > 0 && "invalid pointer size");
-    return LLT{/*isPointer=*/true, /*isVector=*/false,
+    return LLT{/*isPointer=*/true, /*isVector=*/false, /*isScalar=*/false,
                ElementCount::getFixed(0), SizeInBits, AddressSpace};
   }
 
   /// Get a low-level vector of some number of elements and element width.
   static LLT vector(ElementCount EC, unsigned ScalarSizeInBits) {
     assert(!EC.isScalar() && "invalid number of vector elements");
-    assert(ScalarSizeInBits > 0 && "invalid vector element size");
-    return LLT{/*isPointer=*/false, /*isVector=*/true, EC, ScalarSizeInBits,
-               /*AddressSpace=*/0};
+    return LLT{/*isPointer=*/false, /*isVector=*/true, /*isScalar=*/false,
+               EC, ScalarSizeInBits, /*AddressSpace=*/0};
   }
 
   /// Get a low-level vector of some number of elements and element type.
   static LLT vector(ElementCount EC, LLT ScalarTy) {
     assert(!EC.isScalar() && "invalid number of vector elements");
     assert(!ScalarTy.isVector() && "invalid vector element type");
-    return LLT{ScalarTy.isPointer(), /*isVector=*/true, EC,
+    return LLT{ScalarTy.isPointer(), /*isVector=*/true, /*isScalar=*/false,
+               EC,
                ScalarTy.getSizeInBits().getFixedSize(),
                ScalarTy.isPointer() ? ScalarTy.getAddressSpace() : 0};
   }
 
   /// Get a low-level fixed-width vector of some number of elements and element
   /// width.
   static LLT fixed_vector(unsigned NumElements, unsigned ScalarSizeInBits) {
     return vector(ElementCount::getFixed(NumElements), ScalarSizeInBits);
public:
   }
 
   static LLT scalarOrVector(ElementCount EC, uint64_t ScalarSize) {
     assert(ScalarSize <= std::numeric_limits<unsigned>::max() &&
            "Not enough bits in LLT to represent size");
     return scalarOrVector(EC, LLT::scalar(static_cast<unsigned>(ScalarSize)));
   }
 
-  explicit LLT(bool isPointer, bool isVector, ElementCount EC,
+  explicit LLT(bool isPointer, bool isVector, bool isScalar, ElementCount EC,
                uint64_t SizeInBits, unsigned AddressSpace) {
-    init(isPointer, isVector, EC, SizeInBits, AddressSpace);
+    init(isPointer, isVector, isScalar, EC, SizeInBits, AddressSpace);
   }
-  explicit LLT() : IsPointer(false), IsVector(false), RawData(0) {}
+  explicit LLT()
+      : IsScalar(false), IsPointer(false), IsVector(false), RawData(0) {}
 
   explicit LLT(MVT VT);
 
-  bool isValid() const { return RawData != 0; }
+  bool isValid() const { return IsScalar || RawData != 0; }
 
-  bool isScalar() const { return isValid() && !IsPointer && !IsVector; }
+  bool isScalar() const { return IsScalar; }
 
   bool isPointer() const { return isValid() && IsPointer && !IsVector; }
 
   bool isVector() const { return isValid() && IsVector; }
 
   /// Returns the number of elements in a vector LLT. Must only be called on
   /// vector types.
   uint16_t getNumElements() const {
     if (isScalable())
LLT changeElementCount(ElementCount EC) const {
     return LLT::scalarOrVector(EC, getScalarType());
   }
 
   /// Return a type that is \p Factor times smaller. Reduces the number of
   /// elements if this is a vector, or the bitwidth for scalar/pointers. Does
   /// not attempt to handle cases that aren't evenly divisible.
   LLT divide(int Factor) const {
     assert(Factor != 1);
+    assert((!isScalar() || getScalarSizeInBits() != 0) &&
+           "cannot divide scalar of size zero");
     if (isVector()) {
       assert(getElementCount().isKnownMultipleOf(Factor));
       return scalarOrVector(getElementCount().divideCoefficientBy(Factor),
                             getElementType());
     }
 
     assert(getScalarSizeInBits() % Factor == 0);
     return scalar(getScalarSizeInBits() / Factor);
   }
 
   bool isByteSized() const { return getSizeInBits().isKnownMultipleOf(8); }
 
   unsigned getScalarSizeInBits() const {
-    assert(RawData != 0 && "Invalid Type");
-    if (!IsVector) {
-      if (!IsPointer)
-        return getFieldValue(ScalarSizeFieldInfo);
-      else
-        return getFieldValue(PointerSizeFieldInfo);
-    } else {
+    if (IsScalar)
+    if (IsVector) {
       if (!IsPointer)
         return getFieldValue(VectorSizeFieldInfo);
       else
         return getFieldValue(PointerVectorSizeFieldInfo);
-    }
+    } else if (IsPointer)
+      return getFieldValue(PointerSizeFieldInfo);
+    else
+      llvm_unreachable("unexpected LLT");
   }
 
   unsigned getAddressSpace() const {
     assert(RawData != 0 && "Invalid Type");
     assert(IsPointer && "cannot get address space of non-pointer type");
     if (!IsVector)
       return getFieldValue(PointerAddressSpaceFieldInfo);
     else
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   LLVM_DUMP_METHOD void dump() const {
     print(dbgs());
     dbgs() << '\n';
   }
 #endif
 
   bool operator==(const LLT &RHS) const {
     return IsPointer == RHS.IsPointer && IsVector == RHS.IsVector &&
-           RHS.RawData == RawData;
+           IsScalar == RHS.IsScalar && RHS.RawData == RawData;
   }
 
   bool operator!=(const LLT &RHS) const { return !(*this == RHS); }
 
   friend struct DenseMapInfo<LLT>;
   friend class GISelInstProfileBuilder;
 
 private:
   /// LLT is packed into 64 bits as follows:
+  /// isScalar : 1
   /// isPointer : 1
   /// isVector  : 1
-  /// with 62 bits remaining for Kind-specific data, packed in bitfields
+  /// with 61 bits remaining for Kind-specific data, packed in bitfields
   /// as described below. As there isn't a simple portable way to pack bits
   /// into bitfields, here the different fields in the packed structure is
   /// described in static const *Field variables. Each of these variables
   /// is a 2-element array, with the first element describing the bitfield size
   /// and the second element describing the bitfield offset.
   typedef int BitFieldInfo[2];
   ///
   /// This is how the bitfields are packed per Kind:
   /// * Invalid:
   ///   gets encoded as RawData == 0, as that is an invalid encoding, since for
   ///   valid encodings, SizeInBits/SizeOfElement must be larger than 0.
   /// * Non-pointer scalar (isPointer == 0 && isVector == 0):
   ///   SizeInBits: 32;
   static const constexpr BitFieldInfo ScalarSizeFieldInfo{32, 0};
   /// * Pointer (isPointer == 1 && isVector == 0):
   ///   SizeInBits: 16;
   ///   AddressSpace: 24;
   static const constexpr BitFieldInfo PointerSizeFieldInfo{16, 0};
   static const constexpr BitFieldInfo PointerAddressSpaceFieldInfo{
       24, PointerSizeFieldInfo[0] + PointerSizeFieldInfo[1]};
   static_assert((PointerAddressSpaceFieldInfo[0] +
-                 PointerAddressSpaceFieldInfo[1]) <= 62,
+                 PointerAddressSpaceFieldInfo[1]) <= 61,
                 "Insufficient bits to encode all data");
   /// * Vector-of-non-pointer (isPointer == 0 && isVector == 1):
   ///   NumElements: 16;
   ///   SizeOfElement: 32;
   ///   Scalable: 1;
   static const constexpr BitFieldInfo VectorElementsFieldInfo{16, 0};
   static const constexpr BitFieldInfo VectorSizeFieldInfo{
       32, VectorElementsFieldInfo[0] + VectorElementsFieldInfo[1]};
   static const constexpr BitFieldInfo VectorScalableFieldInfo{
       1, VectorSizeFieldInfo[0] + VectorSizeFieldInfo[1]};
-  static_assert((VectorSizeFieldInfo[0] + VectorSizeFieldInfo[1]) <= 62,
+  static_assert((VectorSizeFieldInfo[0] + VectorSizeFieldInfo[1]) <= 61,
                 "Insufficient bits to encode all data");
   /// * Vector-of-pointer (isPointer == 1 && isVector == 1):
   ///   NumElements: 16;
   ///   SizeOfElement: 16;
   ///   AddressSpace: 24;
   ///   Scalable: 1;
   static const constexpr BitFieldInfo PointerVectorElementsFieldInfo{16, 0};
   static const constexpr BitFieldInfo PointerVectorSizeFieldInfo{
       16,
       PointerVectorElementsFieldInfo[1] + PointerVectorElementsFieldInfo[0]};
   static const constexpr BitFieldInfo PointerVectorAddressSpaceFieldInfo{
       24, PointerVectorSizeFieldInfo[1] + PointerVectorSizeFieldInfo[0]};
   static const constexpr BitFieldInfo PointerVectorScalableFieldInfo{
       1, PointerVectorAddressSpaceFieldInfo[0] +
              PointerVectorAddressSpaceFieldInfo[1]};
   static_assert((PointerVectorAddressSpaceFieldInfo[0] +
-                 PointerVectorAddressSpaceFieldInfo[1]) <= 62,
+                 PointerVectorAddressSpaceFieldInfo[1]) <= 61,
                 "Insufficient bits to encode all data");
 
+  uint64_t IsScalar : 1;
   uint64_t IsPointer : 1;
   uint64_t IsVector : 1;
-  uint64_t RawData : 62;
+  uint64_t RawData : 61;
 
   static uint64_t getMask(const BitFieldInfo FieldInfo) {
     const int FieldSizeInBits = FieldInfo[0];
     return (((uint64_t)1) << FieldSizeInBits) - 1;
   }
   static uint64_t maskAndShift(uint64_t Val, uint64_t Mask, uint8_t Shift) {
     assert(Val <= Mask && "Value too large for field");
     return (Val & Mask) << Shift;
   }
   static uint64_t maskAndShift(uint64_t Val, const BitFieldInfo FieldInfo) {
     return maskAndShift(Val, getMask(FieldInfo), FieldInfo[1]);
   }
   uint64_t getFieldValue(const BitFieldInfo FieldInfo) const {
     return getMask(FieldInfo) & (RawData >> FieldInfo[1]);
   }
 
-  void init(bool IsPointer, bool IsVector, ElementCount EC, uint64_t SizeInBits,
-            unsigned AddressSpace) {
+  void init(bool IsPointer, bool IsVector, bool IsScalar, ElementCount EC,
+            uint64_t SizeInBits, unsigned AddressSpace) {
     assert(SizeInBits <= std::numeric_limits<unsigned>::max() &&
            "Not enough bits in LLT to represent size");
     this->IsPointer = IsPointer;
     this->IsVector = IsVector;
-    if (!IsVector) {
-      if (!IsPointer)
-        RawData = maskAndShift(SizeInBits, ScalarSizeFieldInfo);
-      else
-        RawData = maskAndShift(SizeInBits, PointerSizeFieldInfo) |
-                  maskAndShift(AddressSpace, PointerAddressSpaceFieldInfo);
-    } else {
+    this->IsScalar = IsScalar;
+    if (IsScalar)
+    else if (IsVector) {
       assert(EC.isVector() && "invalid number of vector elements");
       if (!IsPointer)
         RawData =
             maskAndShift(EC.getKnownMinValue(), VectorElementsFieldInfo) |
             maskAndShift(SizeInBits, VectorSizeFieldInfo) |
             maskAndShift(EC.isScalable() ? 1 : 0, VectorScalableFieldInfo);
       else
         RawData =
             maskAndShift(EC.getKnownMinValue(),
                          PointerVectorElementsFieldInfo) |
             maskAndShift(SizeInBits, PointerVectorSizeFieldInfo) |
             maskAndShift(AddressSpace, PointerVectorAddressSpaceFieldInfo) |
             maskAndShift(EC.isScalable() ? 1 : 0,
                          PointerVectorScalableFieldInfo);
-    }
+    } else if (IsPointer)
+      RawData = maskAndShift(SizeInBits, PointerSizeFieldInfo) |
+                maskAndShift(AddressSpace, PointerAddressSpaceFieldInfo);
+    else
+      llvm_unreachable("unexpected LLT configuration");
   }
 
 public:
   uint64_t getUniqueRAWLLTData() const {
-    return ((uint64_t)RawData) << 2 | ((uint64_t)IsPointer) << 1 |
-           ((uint64_t)IsVector);
+    return ((uint64_t)RawData) << 3 | ((uint64_t)IsScalar) << 2 |
+           ((uint64_t)IsPointer) << 1 | ((uint64_t)IsVector);
   }
 };
 
 inline raw_ostream& operator<<(raw_ostream &OS, const LLT &Ty) {
   Ty.print(OS);
   return OS;
 }
 

llvm/lib/Support/LowLevelType.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/LowLevelTypeImpl.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 LLT::LLT(MVT VT) {
   if (VT.isVector()) {
-    init(/*IsPointer=*/false, VT.getVectorNumElements() > 1,
+    bool asVector = VT.getVectorNumElements() > 1;
+    init(/*IsPointer=*/false, asVector, /*IsScalar=*/!asVector,
          VT.getVectorElementCount(), VT.getVectorElementType().getSizeInBits(),
          /*AddressSpace=*/0);
   } else if (VT.isValid()) {
     // Aggregates are no different from real scalars as far as GlobalISel is
     // concerned.
-    assert(VT.getSizeInBits().isNonZero() && "invalid zero-sized type");
-    init(/*IsPointer=*/false, /*IsVector=*/false, ElementCount::getFixed(0),
-         VT.getSizeInBits(), /*AddressSpace=*/0);
+    init(/*IsPointer=*/false, /*IsVector=*/false, /*IsScalar=*/true,
+         ElementCount::getFixed(0), VT.getSizeInBits(), /*AddressSpace=*/0);
   } else {
+    IsScalar = false;
     IsPointer = false;
     IsVector = false;
     RawData = 0;
   }
 }
 
 void LLT::print(raw_ostream &OS) const {
   if (isVector()) {

llvm/unittests/CodeGen/LowLevelTypeTest.cpp

 
 using namespace llvm;
 
 namespace {
 
 TEST(LowLevelTypeTest, Scalar) {
   LLVMContext C;
   DataLayout DL("");
 
-  for (unsigned S : {1U, 17U, 32U, 64U, 0xfffffU}) {
+  for (unsigned S : {0U, 1U, 17U, 32U, 64U, 0xfffffU}) {
     const LLT Ty = LLT::scalar(S);
 
     // Test kind.
     ASSERT_TRUE(Ty.isValid());
     ASSERT_TRUE(Ty.isScalar());
 
     ASSERT_FALSE(Ty.isPointer());
     ASSERT_FALSE(Ty.isVector());
 
     // Test sizes.
     EXPECT_EQ(S, Ty.getSizeInBits());
     EXPECT_EQ(S, Ty.getScalarSizeInBits());
 
     // Test equality operators.
     EXPECT_TRUE(Ty == Ty);
     EXPECT_FALSE(Ty != Ty);
 
     // Test Type->LLT conversion.
+    if (S != 0) {
       Type *IRTy = IntegerType::get(C, S);
       EXPECT_EQ(Ty, getLLTForType(*IRTy, DL));
     }
   }
+}
 
 TEST(LowLevelTypeTest, Vector) {
   LLVMContext C;
   DataLayout DL("");
 
-  for (unsigned S : {1U, 17U, 32U, 64U, 0xfffU}) {
+  for (unsigned S : {0U, 1U, 17U, 32U, 64U, 0xfffU}) {
     for (auto EC :
          {ElementCount::getFixed(2), ElementCount::getFixed(3),
           ElementCount::getFixed(4), ElementCount::getFixed(32),
           ElementCount::getFixed(0xff), ElementCount::getScalable(2),
           ElementCount::getScalable(3), ElementCount::getScalable(4),
           ElementCount::getScalable(32), ElementCount::getScalable(0xff)}) {
       const LLT STy = LLT::scalar(S);
       const LLT VTy = LLT::vector(EC, S);
for (auto EC :
       EXPECT_TRUE(VTy == VTy);
       EXPECT_FALSE(VTy != VTy);
 
       // Test inequality operators on..
       // ..different kind.
       EXPECT_NE(VTy, STy);
 
       // Test Type->LLT conversion.
+      if (S != 0) {
         Type *IRSTy = IntegerType::get(C, S);
         Type *IRTy = VectorType::get(IRSTy, EC);
         EXPECT_EQ(VTy, getLLTForType(*IRTy, DL));
       }
     }
   }
+}
 
 TEST(LowLevelTypeTest, ScalarOrVector) {
   // Test version with number of bits for scalar type.
   EXPECT_EQ(LLT::scalar(32),
             LLT::scalarOrVector(ElementCount::getFixed(1), 32));
   EXPECT_EQ(LLT::fixed_vector(2, 32),
             LLT::scalarOrVector(ElementCount::getFixed(2), 32));
   EXPECT_EQ(LLT::scalable_vector(1, 32),