Page MenuHomePhabricator

[PowerPC] Enable redundant TOC save removal on AIX
ClosedPublic

Authored by qiucf on Feb 19 2021, 1:44 AM.

Diff Detail

Event Timeline

qiucf created this revision.Feb 19 2021, 1:44 AM
qiucf requested review of this revision.Feb 19 2021, 1:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2021, 1:44 AM
nemanjai added inline comments.Feb 19 2021, 2:46 AM
llvm/lib/Target/PowerPC/PPCMIPeephole.cpp
229–252

Please add a comment as to why moving the TOC save to the prologue isn't to be done on AIX.

486

On ELFv2, we obviously didn't bother with 32-bit mode because it doesn't really exist. However, on AIX we should probably handle 32-bit mode as well and do this for PPC::STW as well. Then of course, we'll need to update PPCInstrInfo::isTOCSaveMI().

qiucf updated this revision to Diff 325166.Feb 20 2021, 12:05 AM
qiucf marked 2 inline comments as done.
  • Add comment about TOC save in prologue
  • Support 32-bit AIX
hubert.reinterpretcast added inline comments.
llvm/lib/Target/PowerPC/PPCMIPeephole.cpp
229

I think this is not actually not a fundamental limitation of the ABI itself? Can we make the comment more clear that "TOC save in prologue" is merely a strategy that hasn't been implemented for the AIX ABI code in LLVM?

qiucf updated this revision to Diff 325676.Feb 22 2021, 10:02 PM
qiucf marked an inline comment as done.

Update comment with a FIXME.

Gentle ping...

shchenz accepted this revision.Wed, Mar 17, 11:41 PM

LGTM with one minor comment and one idea about further improvement. Thanks for improving this.

llvm/test/CodeGen/PowerPC/remove-redundant-toc-saves.ll
2

Maybe it's better to use AIX64 as prefix comparing with below AIX32?

65

This should be out of this patch's scope, but we should have the opportunity to common TOCSave in block if.then and if.else to their common dominator block entry. Then we should only have one TOCSave in the final assembly.

This revision is now accepted and ready to land.Wed, Mar 17, 11:41 PM
qiucf marked an inline comment as done.Sun, Mar 21, 11:30 PM
qiucf added inline comments.
llvm/test/CodeGen/PowerPC/remove-redundant-toc-saves.ll
65

Thanks for pointing this out. Yes, that's a point for future improvement.

This revision was automatically updated to reflect the committed changes.