[PATCH] D53137: Scalable vector core instruction support + size queries
Diana Picus via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 24 02:26:21 PDT 2019
rovka added a comment.
> I suspect 'ScalableSize' is the wrong term now; 'TypeSize' may be better. Thoughts?
I agree, TypeSize sounds better. Maybe we can replace the public constructor with 2 static methods, TypeSize::Fixed(Size) and TypeSize::Scalable(Size), so we don't always have to spell out /* Scalable =*/.
================
Comment at: llvm/include/llvm/IR/DataLayout.h:454
+ auto BaseSize = getTypeSizeInBits(Ty);
+ return { (BaseSize.getKnownMinSize() + 7) / 8, BaseSize.isScalable() };
}
----------------
We already overload operator /, why not overload + as well so we don't have to change the body of this method?
================
Comment at: llvm/include/llvm/IR/DataLayout.h:487
+ auto BaseSize = getTypeStoreSize(Ty);
+ uint64_t MinAlignedSize = alignTo(BaseSize.getKnownMinSize(),
+ getABITypeAlignment(Ty));
----------------
Can we add a version of alignTo that works with ScalableSize instead?
================
Comment at: llvm/include/llvm/IR/DataLayout.h:656
+ getTypeSizeInBits(VTy->getElementType()).getKnownMinSize();
+ return ScalableSize(MinBits, EltCnt.Scalable);
}
----------------
Maybe just return VTy->getElementCount() * getTypeSizeInBits(VTy->getElementType()).getFixedSize().
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53137/new/
https://reviews.llvm.org/D53137
More information about the llvm-commits
mailing list