[PATCH] D72570: [PowerPC][Future] Prefixed Instructions 64 Byte Boundary Support

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

Other than a few minor nits, LGTM.



================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:14
+// instructions as they are being emitted and align all 8 byte instructions
+// to a 64 byte boundary if possible (by adding a 4 byte nop). This is important
+// because 8 byte instructions are not allowed to cross 64 byte boundaries
----------------
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.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.h:29
+class PPCELFStreamer : public MCELFStreamer {
+  MCSymbol *LastLabel;
+  SMLoc LastLabelLoc;
----------------
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.
```


================
Comment at: llvm/test/MC/PowerPC/ppc64-prefix-align.s:58
+# CHECK-LE-NEXT: c4:	f0 ff 22 38
+LAB1: paddi 1, 2, 8589934576, 0
+paddi 1, 2, 8589934576, 0
----------------
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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72570/new/

https://reviews.llvm.org/D72570