This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Use RegUnits to track register aliases (NFC)
ClosedPublic

Authored by junbuml on Apr 16 2018, 9:59 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

junbuml created this revision.Apr 16 2018, 9:59 AM
thegameg accepted this revision.Apr 16 2018, 5:17 PM
thegameg added a reviewer: t.p.northover.

Thanks for working on this! This looks good to me. Adding Tim for the AArch64LDST part.

This revision is now accepted and ready to land.Apr 16 2018, 5:17 PM

Have you seen LiveRegUnits::accumulate() glancing over the code here (and earlier) it seems to me that TII::tracksRegDefsUses() was just invented to do the same thing: figuring out which registers are free/usable over a range of instructions...

Have you seen LiveRegUnits::accumulate() glancing over the code here (and earlier) it seems to me that TII::tracksRegDefsUses() was just invented to do the same thing: figuring out which registers are free/usable over a range of instructions...

Yes, actually my current change in TII::tracksRegDefsUses() was inspired by LiveRegUnits::accumulate(). The only difference is that we need to track UsedRegs and DefReg separately in TII::tracksRegDefsUses(), while LiveRegUnits::accumulate() track both used/defed registers together. Based on your comment in https://reviews.llvm.org/D41463#inline-400109, would it make sense to introduce new static member function in LiveRegUnits like :

static void LiveRegUnits::accumulateUseDef(MachineInstr &MI, LiveRegUnits &ModifiedRegUnits, LiveRegUnits &UsedRegUnits)  ?

Have you seen LiveRegUnits::accumulate() glancing over the code here (and earlier) it seems to me that TII::tracksRegDefsUses() was just invented to do the same thing: figuring out which registers are free/usable over a range of instructions...

Yes, actually my current change in TII::tracksRegDefsUses() was inspired by LiveRegUnits::accumulate(). The only difference is that we need to track UsedRegs and DefReg separately in TII::tracksRegDefsUses(), while LiveRegUnits::accumulate() track both used/defed registers together. Based on your comment in https://reviews.llvm.org/D41463#inline-400109, would it make sense to introduce new static member function in LiveRegUnits like :

static void LiveRegUnits::accumulateUseDef(MachineInstr &MI, LiveRegUnits &ModifiedRegUnits, LiveRegUnits &UsedRegUnits)  ?

Yes that sounds better!

junbuml updated this revision to Diff 142941.Apr 18 2018, 8:08 AM

Addressed Matthias' comment by moving TargetInstrInfo::trackRegDefsUses into LiveRegUnits::accumulateUsedDefed.

Hi Matthias,
Please let me know if my change in LiveRegUnits is okay with you.
Thanks,
Jun

junbuml updated this revision to Diff 143765.Apr 24 2018, 9:18 AM

Just a minor change in accumulateUsedDefed(). Please let me know any comment.

Kindly ping.
Matthias, please let me know if this change is still okay with you.

sebpop accepted this revision.Apr 27 2018, 9:40 AM

looks good, thanks!

Thanks Sebastian for your review!

This revision was automatically updated to reflect the committed changes.