[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