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

[Yi-Hong Lyu via Phabricator via llvm-commits]

Yi-Hong.Lyu requested changes to this revision.
Yi-Hong.Lyu added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:23
+#include "PPCELFStreamer.h"
+#include "PPCMCCodeEmitter.h"
+#include "PPCInstrInfo.h"
----------------
`PPCMCCodeEmitter.h` should be put later than `PPCInstrInfo.h`


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:45
+                    std::move(Emitter)) {
+  LastLabel = NULL;
+}
----------------
You can just initialized `LastLabel` in member initializer lists so the constructor could be empty.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:106
+    std::unique_ptr<MCObjectWriter> OW, std::unique_ptr<MCCodeEmitter> Emitter,
+    bool RelaxAll) {
+  return new PPCELFStreamer(Context, std::move(MAB), std::move(OW),
----------------
Remove parameter `bool RelaxAll` since it is unused.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:17
 #include "PPCTargetStreamer.h"
+#include "PPCELFStreamer.h"
 #include "TargetInfo/PowerPCTargetInfo.h"
----------------
"PPCELFStreamer.h" should be put before "PPCTargetStreamer.h".


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:112
+  return createPPCELFStreamer(Context, std::move(MAB), std::move(OW),
+                              std::move(Emitter), RelaxAll);
+}
----------------
Argument `RelaxAll` is not necessary since you no longer use it.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.h:70
+  /// This is a prefixed instruction
+  Prefixed = 0x1 << (NewDef_Shift+3)
 };
----------------
Why do you want to shift left `NewDef_Shift+3` instead of `NewDef_Shift+2`?


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

https://reviews.llvm.org/D72570