[PATCH] D68141: [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

Mon Sep 30 11:18:05 PDT 2019

arichardson added inline comments.

================
Comment at: llvm/trunk/lib/Target/AArch64/AArch64StackTagging.cpp:65

-static constexpr unsigned kTagGranuleSize = 16;
+static const Align kTagGranuleSize = Align(16);

----------------
gchatelet wrote:
> arichardson wrote:
> > Can't the Align ctor be constexpr? Will this result in a `Log2` call at runtime?
> The implementation of Log2 is quite complex and depends on the implementation of [[ https://github.com/llvm/llvm-project/blob/02ada9bd2b41d850876a483bede59715e7550c1e/llvm/include/llvm/Support/MathExtras.h#L127 | countLeadingZeros ]].
> GCC, clang, ICC can compute the value at compile time but there may be some compiler that can't.
> https://godbolt.org/z/ytwWHn
> 
> I could solve this issue by introducing a `LogAlign()` function returning an `Align`. This function could be `constexpr`. What do you think? Is it worth the additional complexity?
Since there won't a global ctor at -O2 or higher it probably doesn't matter.

I'm not sure if there have been any of these cases yet, but I would think that having a `LogAlign/Align::fromLog()/etc.` might still be useful for some cases where you get the alignment as log2 instead of in bytes?
This would avoid additional work for the compiler and might be easier to read since you can omit the `1<<N`.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68141/new/

https://reviews.llvm.org/D68141