This is an archive of the discontinued LLVM Phabricator instance.

Power 9 Atomic instructions, load monitored and move XER to CR
ClosedPublic

Authored by nemanjai on Mar 10 2016, 2:46 AM.

Details

Summary

This patch provides the implementation for the following instructions:
lwat,
ldat,
stwat,
stdat,
ldmx
mcrxrx

Diff Detail

Repository
rL LLVM

Event Timeline

nemanjai updated this revision to Diff 50254.Mar 10 2016, 2:46 AM
nemanjai retitled this revision from to Power 9 Atomic instructions, load monitored and move XER to CR.
nemanjai updated this object.
nemanjai added reviewers: kbarton, hfinkel, cycheng, amehsan.
nemanjai set the repository for this revision to rL LLVM.
nemanjai added a subscriber: llvm-commits.
cycheng added inline comments.Mar 10 2016, 11:13 PM
lib/Target/PowerPC/PPCInstr64Bit.td
248

Depending on FC value, input might be $(rD+1), $(rD+2)
I believe that's why we need "hasExtraSrcRegAllocReq".

I am curious about the effect of "hasExtraSrcRegAllocReq", looks like it disallows register changing in AggressiveAntiDepBreaker::ScanInstruction, because it is dangerous to do that for this instruction.

249

Do we need to check $FC value? Or it's user's responsibility?

Even though it looks like user's responsibility:

an Invalid function code will cause the system data storage error handler to be invoked.

920

I don't know when we need to add "isPPC64", Does LDAT and STDAT also require this attribute?

nemanjai added inline comments.Mar 11 2016, 12:28 AM
lib/Target/PowerPC/PPCInstr64Bit.td
248

This is a good point, I added the comment to the 32-bit versions but forgot to add it here. It would certainly be a very bad idea for AADB to rename the target register when we may have values we need in the next register or two. All that being said, this should actually be hasExtraDefRegAllocReq. I'll fix it and re-post.

249

I think that if we decide to provide access to this instruction through a builtin, we should diagnose invalid FC's supplied by the user. However, I don't think attempting to diagnose it in the .td files is the right way to go.

920

You are correct, those instructions are actually 64-bit only instructions. Not just names for the same instructions but that use 64-bit registers rather than 32-bit ones. I'll be sure to add that.

kbarton accepted this revision.Mar 29 2016, 9:22 PM
kbarton edited edge metadata.

LGTM

This revision is now accepted and ready to land.Mar 29 2016, 9:22 PM
nemanjai closed this revision.Mar 31 2016, 8:32 AM

Committed revision 265022.