This mitigates the extra memory caused by D115725.
Thanks! Does this have any impact on binary size, as a proxy for more thorough performance testing?
# no asserts, no debuginfo, -O3 $ du -b clang-stable clang-patched 83539528 clang-stable 83528456 clang-patched
clang's code size actually goes down with this patch
there is a little bit of red for compile times, and a little bit of green for max-rss, I think this tradeoff is fine
Ah, the alignment is 64-bits, so the log2 is 0-63, which fits exactly into 6 bits. That's handy :)
Apparently not, a static assert fires for a 32-bit build of clang. I was assuming 3 was fine based on the PointerIntPair comments.
/// PointerIntPair - This class implements a pair of a pointer and small /// integer. It is designed to represent this in the space required by one /// pointer by bitmangling the integer into the low part of the pointer. This /// can only be done for small integers: typically up to 3 bits, but it depends /// on the number of bits available according to PointerLikeTypeTraits for the /// type.
I suppose we could have a separate 32-bit vs 64-bit implementation of Address, although that's not very nice.
You can get more spare bits by overaligning the type that you're pointing to, if that's suitable/workable. We do that in a few places in LLVM for this sort of reason.
As we're storing pointers to core LLVM types here (Value and Type), I don't think it's possible to over-align them.
Oh well, this probably won't work without some truly squirelly hackery. We store four bits of alignment locally, and make all-zeros a sentinel value to store large alignments out of line. It's not worth it.
without it we end up instantiating AddressImpl<true> unconditionally (because all template parameters are known?) which static_asserts in 32 bit builds
BTW, as a follow-on cleanup, which I may attempt, I think we could convert this alignment from CharUnits to llvm::Align. The vast majority of the getAlignment callers feed it to CGBuilder::CreateAlignedLoad/Store:
Those call IRBuilder::CreateAlignedLoad/Store, which accept MaybeAlign.
This is causing warnings to be emitted:
clang\lib\CodeGen\Address.h(69,1): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) [D:\legalize\llvm\llvm-project\build\tools\clang\lib\CodeGen\obj.clangCodeGen.vcxproj]