[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks added inline comments. Comment at: clang/lib/CodeGen/Address.h:70 +unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt(); +return CharUnits::fromQuantity(1UL << AlignLog); + } LegalizeAdulthood wrote: > This is causing warnings to b

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

LegalizeAdulthood added inline comments. Comment at: clang/lib/CodeGen/Address.h:70 +unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt(); +return CharUnits::fromQuantity(1UL << AlignLog); + } This is causing warnings to be emitted: ``` cla

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb1613f05ae0c: [NFC] Store Address's alignment into PointerIntPairs (authored by aeubanks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

rnk accepted this revision. rnk added a comment. lgtm 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: http://llvm-cs.pcc.me.uk/

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LG from my side Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 __

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks added inline comments. Comment at: clang/lib/CodeGen/Address.h:29 +// so we fallback to storing the alignment separately. +template = 8> class AddressImpl {}; + nikic wrote: > Why do we need the extra T parameter? without it we end up instantiating `Addr

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks updated this revision to Diff 403323. aeubanks added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 Files: clang/lib/CodeGen/Address.h Index: clang/lib/CodeGen/Add

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic added a comment. Approach looks reasonable to me. Comment at: clang/lib/CodeGen/Address.h:29 +// so we fallback to storing the alignment separately. +template = 8> class AddressImpl {}; + Why do we need the extra T parameter? Comment at

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks updated this revision to Diff 403070. aeubanks added a comment. only use PointerIntPair when alignof(Value*) >= 8 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 Files: clang/lib/CodeGen/Addres

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

rnk added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; dblaikie wrote: > nikic wrote: > > dblaikie wrote: > > > aeubanks wrote: > > > > nikic

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

dblaikie added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; nikic wrote: > dblaikie wrote: > > aeubanks wrote: > > > nikic wrote: > > > > Are

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; dblaikie wrote: > aeubanks wrote: > > nikic wrote: > > > Are we guaranteed 3 bits eve

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

dblaikie added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; aeubanks wrote: > nikic wrote: > > Are we guaranteed 3 bits even on 32-bit archite

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; nikic wrote: > Are we guaranteed 3 bits even on 32-bit architectures? Apparently n

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic added inline comments. Comment at: clang/lib/CodeGen/Address.h:45 +auto AlignLog = llvm::Log2_64(alignment.getQuantity()); +assert(AlignLog < (1 << 6) && "cannot fit alignment into 6 bits"); +Pointer.setInt(AlignLog >> 3); nikic wrote: > Why can

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

nikic added inline comments. Comment at: clang/lib/CodeGen/Address.h:30 + // Int portion stores lower 3 bits of the log of the alignment. + llvm::PointerIntPair ElementType; Are we guaranteed 3 bits even on 32-bit architectures? Comment at:

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks added a comment. # no asserts, no debuginfo, -O3 $ du -b clang-stable clang-patched 83539528clang-stable 83528456clang-patched clang's code size actually goes down with this patch llvm-compile-time-tracker: https://llvm-compile-time-tracker.com/compare.php?from=a

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

rnk added a comment. Thanks! Does this have any impact on binary size, as a proxy for more thorough performance testing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 __

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks updated this revision to Diff 399840. aeubanks added a comment. update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117262/new/ https://reviews.llvm.org/D117262 Files: clang/lib/CodeGen/Address.h Index: clang/lib/CodeGen/Address.h ==

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

aeubanks created this revision. aeubanks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This mitigates the extra memory caused by D115725 . Repository: rG LLVM Github Monorepo https://reviews.llvm.or