[PATCH] D140263: [NFC] Vastly simplifies TypeSize
Guillaume Chatelet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 19 06:07:49 PST 2022
gchatelet marked an inline comment as done.
gchatelet added inline comments.
================
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:
> > 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.
> > Thanks. However, MinValue is an implementation detail that should not be exposed even in comments.
>
> Is that true ? I'm under the impression that this is what it was intended for in the original patch
> https://github.com/llvm/llvm-project/commit/b302561b763a1d2eb1a450e135b8d49931936755
>
> ```
> * Adds a TypeSize struct to represent the known minimum size of a type
> along with a flag to indicate that the runtime size is a integer multiple
> of that size
> ```
Let me rephrase "yes you're right that this is an implementation detail and it shouldn't be exposed but in that particular case it seems to be the exact semantic for the type".
I think it's better to update the documentation to reflect what was in the original comment. WDYT?
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