This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Update PC-Relative Load/Store Patterns to use the refactored Load/Store Implementation
ClosedPublic

Authored by amyk on Jan 20 2021, 11:01 PM.

Details

Summary

This patch updates the PC-Relative load and store patterns to utilize the refactored load/store implementation introduced in D93370.
PC-Relative implementation has been added to PPCISelLowering.cpp, and also the patterns in PPCInstrPrefix.td have been updated and no longer require AddedComplexity.
All existing test cases pass with this update.

Depends on D95115

Diff Detail

Event Timeline

amyk created this revision.Jan 20 2021, 11:01 PM
amyk requested review of this revision.Jan 20 2021, 11:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2021, 11:01 PM
bsaleil accepted this revision.Jan 22 2021, 3:44 PM
bsaleil added a subscriber: bsaleil.

LGTM, I only have a minor comment.

llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
267

Could you rename the P argument to Parent like in the other Select* functions ?

This revision is now accepted and ready to land.Jan 22 2021, 3:44 PM
amyk updated this revision to Diff 320767.Feb 2 2021, 6:05 AM

Update patch to:

  • rebase patch
  • Update the naming of the variable to Parent
amyk updated this revision to Diff 323318.Feb 12 2021, 6:57 AM

Update patch to rebase from latest change on D93370.

amyk updated this revision to Diff 323860.Feb 15 2021, 6:06 PM

Update patch based on dependent patches, and update a comment when checking for P10 specific flags.

amyk updated this revision to Diff 330640.Mar 15 2021, 6:43 AM

Update patch based on dependent patch (D95115) update.

amyk updated this revision to Diff 339839.Apr 22 2021, 7:14 PM
amyk retitled this revision from [NFC][PowerPC] Update PC-Relative Load/Store Patterns to use the refactored Load/Store Implementation to [PowerPC] Update PC-Relative Load/Store Patterns to use the refactored Load/Store Implementation.
  • Rebase patch with respect to latest updates in D95115
  • Remove NFC from the title to add extra PC-Relative test cases (involving atomic load/stores and special cases involving PSTXSDpc that were not tested previously
amyk added a comment.May 7 2021, 7:45 AM

Gentle ping.

nemanjai accepted this revision.May 13 2021, 4:17 AM

LGTM.

amyk updated this revision to Diff 364998.Aug 7 2021, 8:39 PM

Discussed offline with Nemanja to simplify the patch slightly and removing the MOF_PCRel flag, as well as adding an assert.

amyk added a comment.Aug 9 2021, 9:39 AM

@nemanjai I've updated the PC-Relative patch once more, if you could take another look when you get the chance to see if I have addressed the comments that we discussed about offline, that would be great.

nemanjai accepted this revision.Sep 7 2021, 8:11 AM

LGTM other than a couple of minor nits.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
17361

Nit: please avoid the

if (condition)
  return true;
return false;

idiom. This is simply return condition;

17614

I realize that we don't really use Base but we should either note that in a comment or just set it to some value that means "we don't care".

llvm/lib/Target/PowerPC/PPCInstrInfo.td
1154–1155

Is this no longer needed?

amyk added inline comments.Sep 9 2021, 7:57 AM
llvm/lib/Target/PowerPC/PPCInstrInfo.td
1154–1155

Yeah, it is no longer needed. I can remove this when I commit the patch.

amyk updated this revision to Diff 371600.Sep 9 2021, 7:59 AM

Address nit review comments from Nemanja and remove the pcreladdr ComplexPattern (as it is no longer in use).

This revision was landed with ongoing or failed builds.Sep 9 2021, 1:40 PM
This revision was automatically updated to reflect the committed changes.