[PATCH] D77394: [Alignment][NFC] Add DebugStr and operator*
Clement Courbet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Apr 5 23:57:42 PDT 2020
courbet accepted this revision.
courbet added inline comments.
This revision is now accepted and ready to land.
================
Comment at: llvm/include/llvm/Support/Alignment.h:398
+inline Align operator*(Align Lhs, uint64_t Rhs) {
+ assert(Rhs > 0 && "Rhs must be positive");
----------------
gchatelet wrote:
> courbet wrote:
> > courbet wrote:
> > > What is the meaning of `Align(1) * 3` ?
> > My point is, I think what you want is an `operator<<()`
> Since we return an `Align` (or `MaybeAlign`) the result have to be a power of two (or it will assert).
> I can make it clear by requesting a power of two for `Rhs`.
>
> I agree that a shift operator has the correct semantic and is less error prone but then the call site would be harder to understand from
>
> ```
> MTI->setDestAlignment(I.getDestAlign() * DFSF.DFS.ShadowWidthBytes);
> MTI->setSourceAlignment(I.getSourceAlign() * DFSF.DFS.ShadowWidthBytes);
> ```
> to
> ```
> MTI->setDestAlignment(I.getDestAlign() << Log2_64(DFSF.DFS.ShadowWidthBytes));
> MTI->setSourceAlignment(I.getSourceAlign() << Log2_64(DFSF.DFS.ShadowWidthBytes));
> ```
>
> WDYT?
Alright, makes sense.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77394/new/
https://reviews.llvm.org/D77394
More information about the llvm-commits
mailing list