Page MenuHomePhabricator

[PowerPC][Future] Prefixed Instructions 64 Byte Boundary Support
ClosedPublic

Authored by stefanp on Jan 12 2020, 8:11 AM.

Details

Summary

A known limitation for Future CPU is that the new prefixed instructions may not cross 64 Byte boundaries.
All instructions are already 4 byte aligned so the only situation where this can occur is when the prefix is in one 64 byte block and the instruction that is prefixed is at the top of the next 64 byte block. To fix this case PPCELFStreamer was added to intercept EmitInstruction. When a prefixed instruction is emitted we try to align it to 64 Bytes by adding a maximum of 4 bytes. If the prefixed instruction crosses the 64 Byte boundary then the alignment would trigger and a 4 byte nop would be added to push the instruction into the next 64 byte block.

Diff Detail

Event Timeline

stefanp created this revision.Jan 12 2020, 8:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2020, 8:11 AM
nemanjai accepted this revision.Jan 12 2020, 9:23 AM

Other than a few minor nits, LGTM.

llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp
15

Maybe this should
s/if possible/if required
I suppose you are trying to communicate that we will only align an instruction on a 64B boundary if it is crossing the boundary.

llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.h
30

Perhaps a comment about why we need this:

// We need to keep track of the last label we emitted (only one) because
// depending on whether the label is on the same line as an aligned
// instruction or not, the label may refer to the instruction or the nop.
llvm/test/MC/PowerPC/ppc64-prefix-align.s
59

Would it be possible to add branches to LAB1 and LAB2 to show that one of them will branch to the nop and the other one won't?

This revision is now accepted and ready to land.Jan 12 2020, 9:23 AM
stefanp updated this revision to Diff 237548.Jan 12 2020, 10:06 AM

Added full context.

jsji added a reviewer: Restricted Project.Jan 13 2020, 7:31 AM
jsji added a project: Restricted Project.
stefanp updated this revision to Diff 237730.Jan 13 2020, 10:47 AM

Address review comments.

Yi-Hong.Lyu requested changes to this revision.Jan 20 2020, 11:49 AM
Yi-Hong.Lyu added a subscriber: Yi-Hong.Lyu.
Yi-Hong.Lyu added inline comments.
llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp
24

PPCMCCodeEmitter.h should be put later than PPCInstrInfo.h

46

You can just initialized LastLabel in member initializer lists so the constructor could be empty.

107

Remove parameter bool RelaxAll since it is unused.

llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp
18

"PPCELFStreamer.h" should be put before "PPCTargetStreamer.h".

110

Argument RelaxAll is not necessary since you no longer use it.

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

Why do you want to shift left NewDef_Shift+3 instead of NewDef_Shift+2?

This revision now requires changes to proceed.Jan 20 2020, 11:49 AM
stefanp updated this revision to Diff 240741.Jan 27 2020, 5:37 PM

Address review comments.

Yi-Hong.Lyu accepted this revision.Jan 28 2020, 6:12 PM
This revision is now accepted and ready to land.Jan 28 2020, 6:12 PM
This revision was automatically updated to reflect the committed changes.