huntergr added inline comments.
================
Comment at: llvm/include/llvm/IR/DataLayout.h:454
+ auto BaseSize = getTypeSizeInBits(Ty);
+ return { (BaseSize.getKnownMinSize() + 7) / 8, BaseSize.isScalable() };
}
----------------
rovka wrote:
> We already overload operator /, why not overload + as well so we don't have
> to change the body of this method?
Scaling a size with * or / has a clear meaning to me, since it's independent of
vscale; getting a vector that's half the size or four times larger just works.
Using + (or -) on the other hand doesn't seem to be as clear; I wasn't sure if
a standalone int should be automatically treated as being the same as the
TypeSize, or always considered Fixed. If we try for the former I can imagine
quite a few bugs arising.
I could add a roundBitsToNearestByteSize method to move the arithmetic
elsewhere if that would be acceptable?
================
Comment at: llvm/include/llvm/IR/DataLayout.h:656
+
getTypeSizeInBits(VTy->getElementType()).getKnownMinSize();
+ return ScalableSize(MinBits, EltCnt.Scalable);
}
----------------
rovka wrote:
> Maybe just return VTy->getElementCount() *
> getTypeSizeInBits(VTy->getElementType()).getFixedSize().
There's no support for generating a TypeSize from an ElementCount in that way;
is that an interface you feel is useful?
(I'll certainly change the `getKnownMinSize` to `getFixedSize` though, since
we're just referring to a scalar)
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
