[PATCH] [AArch64] FP load balancing pass for Cortex-A57

[David Blaikie]

On Fri, Aug 8, 2014 at 5:43 AM, James Molloy <james at jamesmolloy.co.uk> wrote:
> Thanks Tim,
>
> Using unique_ptr ended up making the code harder to read, so I simply added
> a deletion loop which I think is cleaner.

I haven't looked at this patch in detail, but is there a reasonable
summary of why unique_ptr made the code harder to read?

I'm really interested in making/encouraging ownership to be more
explicit/clear and would sometimes even encourage it when it hurts
(superficial (which might not be the case here)) readability. It's
sometimes the case that ownership looks awkward when you make it
explicit because it's an awkward ownership scheme and making it
implicit just hinds that complexity (in the not good way of hiding -
making it a trap/difficult for others to correctly modify the code in
the future).

- David

>
> Committed in r215199, thankyou to all reviewers!
>
> Cheers,
>
> James
>
>
> On 8 August 2014 10:42, Tim Northover <t.p.northover at gmail.com> wrote:
>>
>> Just added llvm-commits.
>>
>> ================
>> Comment at: lib/Target/AArch64/AArch64A57FPLoadBalancing.cpp:318
>> @@ +317,3 @@
>> +  // "link" register between each inst in the chain.
>> +  std::map<unsigned, Chain*> ActiveChains;
>> +  std::set<Chain*> AllChains;
>> ----------------
>> Tim Northover wrote:
>> > Aren't these chains leaked? It *looks* like you could make this (or the
>> > set) a unique_ptr<Chain>.
>> Also, this should definitely be the set. I hadn't noticed things get
>> removed from ActiveChains when I wrote this. Should have been obvious, in
>> retrospect.
>>
>> http://reviews.llvm.org/D4791
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>