[PATCH] D140263: [NFC] Vastly simplifies TypeSize
Sergei Barannikov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 18 08:22:20 PST 2022
barannikov88 added a comment.
Mostly fine by me, but I think the initial author should take a look.
================
Comment at: llvm/include/llvm/Support/TypeSize.h:114
friend LeafTy &operator+=(LeafTy &LHS, const LeafTy &RHS) {
- assert(LHS.UnivariateDim == RHS.UnivariateDim && "Invalid dimensions");
- LHS.Value += RHS.Value;
+ assert(LHS.Scalable == RHS.Scalable && "Invalid dimensions");
+ LHS.MinValue += RHS.MinValue;
----------------
Here, and in the method below, the error message is outdated.
================
Comment at: llvm/include/llvm/Support/TypeSize.h:170
/// Returns the minimum value this size can represent.
+ ScalarTy getKnownMinValue() const { return MinValue; }
----------------
This class is no longer "size", it is now "quantity". Either the comments should be fixed, or the class name.
================
Comment at: llvm/include/llvm/Support/TypeSize.h:168
- /// Add \p RHS to the value at the univariate dimension.
- LeafTy getWithIncrement(ScalarTy RHS) const {
- return static_cast<LeafTy>(
- UnivariateLinearPolyBase(Value + RHS, UnivariateDim));
+ /// Add \p Rhs to the value at the univariate dimension.
+ LeafTy getWithIncrement(ScalarTy Rhs) const {
----------------
gchatelet wrote:
> barannikov88 wrote:
> > univariate dimension --> fixed quantity?
> >
> Reworded it MinValue since this is what it is.
Thanks. However, MinValue is an implementation detail that should not be exposed even in comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140263/new/
https://reviews.llvm.org/D140263
More information about the llvm-commits
mailing list