This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] generate CTR loops instructions after ISEL
ClosedPublic

Authored by shchenz on Mar 21 2022, 3:34 AM.

Details

Summary

This was first raised in https://reviews.llvm.org/D81353#2078725.

We solved many bugs related to PowerPC generating CTR Loops too early and after ISEL we found there is CTR clobber in the CTR loop. So we have to exclude the CTR clobber instructions in hardware loop insertion pass one by one.

This patch implements a new way to generate the CTR loops. Now the intrinsics inserted in hardware loop pass will be mapped to pseudo instructions and these pseudo instructions will be expanded to CTR loop or normal compare+branch loop based on the result of a new added post-isel pass PPCCTRLoop.

This is the first part, including:
1: pseudo instructions for the CTR loops.
2: a new pass to expand the pseudo instructions.

One or more follow up patches will map the hardware loop intrinsics to the new added pseudo instructions and delete the CTR clobber check in hardware loop insertion pass.

Diff Detail

Event Timeline

shchenz created this revision.Mar 21 2022, 3:34 AM
shchenz requested review of this revision.Mar 21 2022, 3:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2022, 3:34 AM
shchenz updated this revision to Diff 416887.Mar 21 2022, 4:08 AM
qiucf added a subscriber: qiucf.Mar 24 2022, 1:43 AM
qiucf added inline comments.
llvm/lib/Target/PowerPC/PPCCTRLoops.cpp
193

Nit: \n is not needed in assert message.

shchenz planned changes to this revision.Mar 29 2022, 3:32 AM
shchenz updated this revision to Diff 421441.EditedApr 8 2022, 12:43 AM

update after verifying together with functionality patch D123366

shchenz marked an inline comment as done.Apr 8 2022, 12:55 AM
shchenz added inline comments.
llvm/lib/Target/PowerPC/PPCCTRLoops.cpp
193

thanks!

shchenz marked an inline comment as done.Apr 19 2022, 6:14 PM

gentle ping

gentle ping

lkail added a subscriber: lkail.May 18 2022, 3:53 PM
lkail added inline comments.
llvm/lib/Target/PowerPC/PPCCTRLoops.cpp
108

Can we use modifiesRegsiter here to further check RegMask clobbering?

115

Why would a Call read CTR register?

154

Can we rename Revert to something like InvalidCTRLoop? Revert sounds like we have performed part of transformation, then rollback.

158

Typo: either

llvm/test/CodeGen/PowerPC/ctrloops64.mir
240

Please add test whose loop has calls into other functions.

lkail added inline comments.May 18 2022, 7:24 PM
llvm/lib/Target/PowerPC/PPCCTRLoops.cpp
53

Do we have to add stats to count how many loops transformed?

178

nit: isCTRClobber(&*I, /* CheckReads */ false)

185

nit: isCTRClobber(&*I, /* CheckReads */ true)

197

nit: isCTRClobber(&MI, /* CheckReads */ true)

214

It's interesting you define Is32Bit. Most PowerPC code tend to define IsPPC64 something like that.

shchenz updated this revision to Diff 431283.May 22 2022, 9:02 PM
shchenz marked 9 inline comments as done.
shchenz added a reviewer: lkail.

address @lkail comments

Thanks for review @lkail , updated accordingly.

llvm/lib/Target/PowerPC/PPCCTRLoops.cpp
115

CTR is a volatile register, this is not for the Call itself, it is also for the callee function. Inside the callee, we may read the CTR with mfctr or other similar instructions.

But Call may also clobber the CTR, we should check isCall before !CheckReads

154

Sure, good idea.

lkail added inline comments.May 22 2022, 9:27 PM
llvm/lib/Target/PowerPC/PPCCTRLoops.cpp
100

nit: Check isOutermost.

115

Normal call should be covered by checking regmask. Another thing I'm concerned about is indirect branch by bctr. We should add test to ensure bailing out if indirect branch is involved.

shchenz updated this revision to Diff 432822.May 29 2022, 7:57 PM

add a new case for indirect call.

shchenz marked 2 inline comments as done.May 29 2022, 7:57 PM

Thanks for review @lkail updated.

jsji resigned from this revision.Jun 2 2022, 7:51 AM

gentle ping

Hi @lkail , could you please help to have another look? Thanks.

lkail accepted this revision as: lkail.Jun 19 2022, 11:25 PM

LGTM.

This revision is now accepted and ready to land.Jun 19 2022, 11:25 PM
This revision was landed with ongoing or failed builds.Jun 20 2022, 7:57 PM
This revision was automatically updated to reflect the committed changes.