[PATCH] D77394: [Alignment][NFC] Add DebugStr and operator*
Guillaume Chatelet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 3 08:02:36 PDT 2020
gchatelet marked an inline comment as done.
gchatelet added inline comments.
================
Comment at: llvm/include/llvm/Support/Alignment.h:398
+inline Align operator*(Align Lhs, uint64_t Rhs) {
+ assert(Rhs > 0 && "Rhs must be positive");
----------------
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?
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