[PATCH] D32530: [SVE][IR] Scalable Vector IR Type

[Graham Hunter via Phabricator via llvm-commits]

huntergr added inline comments.


================
Comment at: lib/IR/LLVMContextImpl.h:1311
   DenseMap<std::pair<Type *, uint64_t>, ArrayType*> ArrayTypes;
-  DenseMap<std::pair<Type *, unsigned>, VectorType*> VectorTypes;
+  DenseMap<std::pair<Type *, uint64_t>, VectorType*> VectorTypes;
   DenseMap<Type*, PointerType*> PointerTypes;  // Pointers in AddrSpace = 0
----------------
rkruppe wrote:
> Nit: have you considered `std::pair<bool, unsigned>` instead of manually bit-packing it into 64 bits? DenseMap should support nested pairs, the size should be the same (except if unsigned is 64 bit, which I don't believe we support and which is extremely niche anyway), and it would simplify `VectorType::get` a little bit.
So I tried this, and couldn't compile it -- there's no implementation of getHashValue, getEmptyKey, getTombstoneKey, etc. for the nested pair.

In our downstream compiler this is actually implemented as a dense map of a 3-element tuple against the Type*, for which we have implemented the appropriate extensions to DenseMapInfo.

If that approach is preferred to bitpacking, I'll make a separate patch to implement the DenseMap extensions.


https://reviews.llvm.org/D32530