[llvm] r211097 - Fix memory leak of RegScavenger accidentally added in r211037.

David Blaikie dblaikie at gmail.com
Tue Jun 17 10:44:55 PDT 2014


On Tue, Jun 17, 2014 at 6:02 AM, Tim Northover <t.p.northover at gmail.com> wrote:
>> +    delete RS;
>
> I think there's a strong case for std::unique_ptr here.

+1

Or, better yet, is there any reason the RegScavenger is dynamically
allocated at all?

I assume this could just be a RegScavenger member and just
re-initialize it on every call to runOnMachineFunction:

RS = RegScavenger();

Alternatively, you could flip this API around and just have a stack
object for all of that state (TM, TL, AFI, etc... ):

  LSO thing(Fn);

  for (MBB : Fn) {
    Modified |= thing.LodaStoreMultipleOpti(MBB);
    if (...)
      Modified |= thing.MergeReturnIntoLDM(MBB);
  }

  return Modified;

Then you wouldn't have to worry about introducing a new piece of state
and failing to reset it every time...

Chandler: Does the new pass manager make this any more likely to be
the obvious way to write a pass? It seems silly to have such trivial
state be reset every time when just having a per-execution scope (like
'thing' above) expresses the intended semantic so well. (though as
we've discussed before, sometimes that's a performance saving -
because you've got some big container that you'd rather clear and then
reuse the memory than throw away and start again on each function)

- David



More information about the llvm-commits mailing list