This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC][NFC] Add a function to determine if a call needs to be NOTOC.
ClosedPublic

Authored by stefanp on Apr 25 2022, 1:37 PM.

Details

Summary

Add the isNoTOCCallInstr function to PPCInstrInfo to determine if a call opcode
does not need a TOC restore after the call. All call opcodes should be listed in
this function. A default unreachable in this function should force future call
opcodes to also be added.

This is a follow up patch to D122012

Diff Detail

Event Timeline

stefanp created this revision.Apr 25 2022, 1:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2022, 1:37 PM
stefanp requested review of this revision.Apr 25 2022, 1:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2022, 1:37 PM
stefanp added reviewers: lei, nemanjai, jsji, Restricted Project.Apr 25 2022, 1:38 PM
shchenz added inline comments.Apr 25 2022, 10:42 PM
llvm/lib/Target/PowerPC/PPCInstrInfo.h
299

I am in favor of Jinsong's comment in https://reviews.llvm.org/D122012#3392673. Could we use a flag in the td file (with a default true/false value) instead of listing all call instructions in the cpp file?

jsji accepted this revision as: jsji.Apr 26 2022, 6:36 AM

LGTM. Thanks.

llvm/lib/Target/PowerPC/PPCInstrInfo.h
299

Thanks Zheng, we had some offline discussion, and I agreed that this can be a tradeoff between cost and benefits.
So I am OK with this.

This revision is now accepted and ready to land.Apr 26 2022, 6:36 AM
shchenz added inline comments.Apr 26 2022, 6:44 AM
llvm/lib/Target/PowerPC/PPCInstrInfo.h
299

Oh, I don't realize this. Could you please explain why we choose the cpp function instead of a td flag? To me, when we add a new call instruction in td file in future, it should be easier to aware that there is a flag to tell whether this call instruction needs a nop or not. Thanks

jsji added inline comments.Apr 26 2022, 6:49 AM
llvm/lib/Target/PowerPC/PPCInstrInfo.h
299

We will need some big in TSFlags to check the td flag, and currently this only affects 3 opcode, and it is highly unlikely we will be adding more NOTOC call instructions.
So this explicitly listed switch table is mostly as a tradeoff of saving bits in TSFlags

shchenz accepted this revision as: shchenz.Apr 26 2022, 6:23 PM

LGTM with one nit.

llvm/lib/Target/PowerPC/PPCInstrInfo.h
299

OK, thanks for explanation.

314

nit: maybe we can also guard these false cases under #ifndef NDEBUG?

stefanp updated this revision to Diff 426041.Apr 29 2022, 6:33 AM

Rebased patch to top of trunk.
Added the additional #ifndef NDEBUG.

This revision was landed with ongoing or failed builds.Apr 29 2022, 6:36 AM
This revision was automatically updated to reflect the committed changes.