[RFC] Remove User::OperandList

Pete Cooper peter_cooper at apple.com
Fri May 29 10:29:51 PDT 2015


Hey Owen
> On May 29, 2015, at 10:26 AM, Owen Anderson <resistor at mac.com> wrote:
> 
> Hi Pete,
> 
> I like this overall, though I haven’t had the time to look at all of the changes yet.  One thing jumped out at me:
Thanks for taking a look.
> 
>> Note, the bit used here was taken from SubclassOptionalData of which only FastMathFlags needed 5 bits it was safe to reduce SubclassOptionalData to 6.
> 
> Could we use a PointerIntPair on the UseList pointer instead?  I’d hate to run out of bits in SubclassOptionalData some time in the future because we stole one here.
Yeah, my original implementation actually did just this (can’t remember if i did UseList or Type, but one of the 2).  I wasn’t sure if there would be a significant performance regression or not to using a PointerIntPair so I stole the optional bit instead.  I’m totally fine with the PointerIntPair solution though.  Having measured the performance more recently, there was no noticeable change.

Cheers,
Pete
> 
> —Owen
> 
>> On May 28, 2015, at 11:30 AM, Pete Cooper <peter_cooper at apple.com> wrote:
>> 
>> Hi all
>> 
>> This series of patches removes around 2.5% of peak memory by computing OperandList instead of always storing it in User.
>> 
>> The majority of the patches are cleanup and refactoring to make OperandList and NumOperands be managed entirely in User and not have subclasses changing their values directly.  The first few patches also move all the ‘hung off uses’ logic for PhiNode/SwitchInst/LandingPadInst/IndirectBr in to User as that code was particularly heavy on hacking on the OperandList.
>> 
>> The OperandList itself was never actually required for all subclasses of User.  The hung off cases needed it as they actually allocate memory for operands, but the majority of instructions for example allocate a fixed size array of Use intrusively in User::new.  We can get to the first operand for these instructions by subtracting (NumOperands * sizeof(Use)) from the User this pointer.  We use the pointer arithmetic on these cases, and in the hung off case we allocate the Use* in User::new instead of the Use[N] array.  This requires that we have another User::new for hung off uses.
>> 
>> The main change here is actually in 0008 which changes getOperandList() from
>> 
>> Use *getOperandList() const {
>> return LegacyOperandList;
>> }
>> 
>> to
>> 
>> const Use *getOperandList() const {
>>   if (HasHungOffUses) {
>>     Use * const*Storage = reinterpret_cast<Use* const*>(this) - 1;
>>     return *Storage;
>>   }
>>   const Use *Storage = reinterpret_cast<const Use*>(this) - NumOperands;
>>   return Storage;
>> }
>> 
>> The numbers are also in 0008, but to give them here:
>> 
>> On 'opt -O2 verify-uselistorder.lto.bc', peak memory usage prior to this change is 149MB and after is 143MB so the savings are around 2.5% of peak.
>> 
>> Looking at some passes which allocate many Instructions and Values, parseIR drops from 54.25MB to 52.21MB while the Inliner calls to Instruction::clone() drops from 28.20MB to 27.05MB.
>> 
>> Comments welcome.
>> 
>> Cheers,
>> Pete
>> 
>> <0001-Move-the-special-Phi-logic-for-hung-off-uses-in-to-U.patch><0002-Make-User-track-whether-a-class-has-hung-off-uses-an.patch><0003-Delete-User-dropHungOffUses-and-move-it-in-to-User-w.patch><0004-Add-User-reallocHungoffUses-and-use-it-to-grow-the-h.patch><0005-Stop-returning-a-Use-from-allocHungOffUses.patch><0006-Added-a-version-of-User-new-for-hung-off-uses.patch><0007-Replace-all-accesses-to-User-OperandList-with-getter.patch><0008-Move-OperandList-to-be-allocated-prior-to-User-for-h.patch><0009-Rename-NumOperands-to-make-it-clear-its-managed-by-t.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 





More information about the llvm-commits mailing list