[PATCH] D53137: Scalable vector core instruction support + size queries

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen accepted this revision.
sdesmalen added a comment.

Thanks @huntergr , I think this interface looks really nice. LGTM!



================
Comment at: llvm/include/llvm/IR/DataLayout.h:456
+  TypeSize getTypeStoreSize(Type *Ty) const {
+    auto BaseSize = getTypeSizeInBits(Ty);
+    return { (BaseSize.getKnownMinSize() + 7) / 8, BaseSize.isScalable() };
----------------
nit: use TypeSize instead of auto.


================
Comment at: llvm/include/llvm/IR/DataLayout.h:622
   case Type::LabelTyID:
-    return getPointerSizeInBits(0);
+    return TypeSize::Fixed(getPointerSizeInBits(0));
   case Type::PointerTyID:
----------------
Nice, I like the interface of `TypeSize::Fixed()` for fixed-size types.


================
Comment at: llvm/lib/IR/Instructions.cpp:2990
   // Get the bit sizes, we'll need these
-  unsigned SrcBits = SrcTy->getPrimitiveSizeInBits();   // 0 for ptr
-  unsigned DestBits = DestTy->getPrimitiveSizeInBits(); // 0 for ptr
+  auto SrcBits = SrcTy->getPrimitiveSizeInBits();   // 0 for ptr
+  auto DestBits = DestTy->getPrimitiveSizeInBits(); // 0 for ptr
----------------
nit: auto -> TypeSize ?


================
Comment at: llvm/lib/IR/Instructions.cpp:3052
 
-  unsigned SrcBits = SrcTy->getPrimitiveSizeInBits();   // 0 for ptr
-  unsigned DestBits = DestTy->getPrimitiveSizeInBits(); // 0 for ptr
+  auto SrcBits = SrcTy->getPrimitiveSizeInBits();   // 0 for ptr
+  auto DestBits = DestTy->getPrimitiveSizeInBits(); // 0 for ptr
----------------
nit: auto -> TypeSize ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53137/new/

https://reviews.llvm.org/D53137



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits