[llvm] r265842 - Codegen: Factor tail duplication into a utility class. NFC

[Kyle Butt via llvm-commits]

Actually, in looking at how it will be used in MachineBlockPlacement, it
makes more sense to have an init method.

The reason is that the duplicator needs to be set up once per machine
function, and then used far down the call stack during placement.
Other solutions would involve something like std::unique_ptr, and either
heap allocating the Duplicator, or storing a stack allocated Duplicator
somewhere in the object.

I think overall it's cleaner just to have an init method.

I don't see any reason it can't be on the stack during the TailDuplication
pass, if you find that cleaner.

On Mon, Apr 11, 2016 at 3:48 PM, Kyle Butt <iteratee at google.com> wrote:

>
>
> On Mon, Apr 11, 2016 at 3:32 PM, Justin Bogner <mail at justinbogner.com>
> wrote:
>
>> Kyle Butt via llvm-commits <llvm-commits at lists.llvm.org> writes:
>> > Author: iteratee
>> > Date: Fri Apr  8 15:35:01 2016
>> > New Revision: 265842
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=265842&view=rev
>> > Log:
>> > Codegen: Factor tail duplication into a utility class. NFC
>> >
>> > This is in preparation for tail duplication during block placement. See
>> D18226.
>> > This needs to be a utility class for 2 reasons. No passes may run after
>> block
>> > placement, and also, tail-duplication affects subsequent layout
>> decisions, so
>> > it must be interleaved with placement, and can't be separated out into
>> its own
>> > pass. The original pass is still useful, and now runs by delegating to
>> the
>> > utility class.
>> >
>> > Added:
>> >     llvm/trunk/include/llvm/CodeGen/TailDuplicator.h
>> >     llvm/trunk/lib/CodeGen/TailDuplicator.cpp
>> >       - copied, changed from r265841,
>> llvm/trunk/lib/CodeGen/TailDuplication.cpp
>> > Modified:
>> >     llvm/trunk/lib/CodeGen/CMakeLists.txt
>> >     llvm/trunk/lib/CodeGen/TailDuplication.cpp
>>
>> We probably don't really need TailDuplicator and TailDuplication to be
>> in different source files - the TailDuplication is basically just a few
>> lines at this point and could pretty easily fit at the bottom of the
>> source file with the utilities.
>>
> > Modified: llvm/trunk/lib/CodeGen/TailDuplication.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TailDuplication.cpp?rev=265842&r1=265841&r2=265842&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/lib/CodeGen/TailDuplication.cpp (original)
>> > +++ llvm/trunk/lib/CodeGen/TailDuplication.cpp Fri Apr  8 15:35:01 2016
>> > @@ -8,147 +8,52 @@
>>  ...
>> >  bool TailDuplicatePass::runOnMachineFunction(MachineFunction &MF) {
>> >    if (skipOptnoneFunction(*MF.getFunction()))
>> >      return false;
>> >
>> > -  TII = MF.getSubtarget().getInstrInfo();
>> > -  TRI = MF.getSubtarget().getRegisterInfo();
>> > -  MRI = &MF.getRegInfo();
>> > -  MMI = getAnalysisIfAvailable<MachineModuleInfo>();
>> > -  MBPI = &getAnalysis<MachineBranchProbabilityInfo>();
>> > -
>> > -  PreRegAlloc = MRI->isSSA();
>> > -  RS.reset();
>> > -  if (MRI->tracksLiveness() && TRI->trackLivenessAfterRegAlloc(MF))
>> > -    RS.reset(new RegScavenger());
>> > +  auto MMI = getAnalysisIfAvailable<MachineModuleInfo>();
>> > +  auto MBPI = &getAnalysis<MachineBranchProbabilityInfo>();
>> > +
>> > +  Duplicator.initMF(MF, MMI, MBPI);
>>
>> Why is this a member rather than just being on the stack here?
>> Duplicator.initMF() fully reinitializes the object anyway, so it's not
>> like we're avoiding some expensive setup or anything.
>>
>> This would also let us get rid of initMF and do the initialization in a
>> constructor, which is simpler.
>>
>
> I agree. I'll make it a constructor.
>
>
>>
>> >    bool MadeChange = false;
>> > -  while (TailDuplicateBlocks(MF))
>> > +  while (Duplicator.tailDuplicateBlocks(MF))
>> >      MadeChange = true;
>> >
>> >    return MadeChange;
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160412/024fde80/attachment.html>