[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