[PATCH] D72569: [PowerPC][Future] Add prefixed instruction paddi to future CPU

[Kamau Bridgeman via Phabricator via llvm-commits]

kamaub added a comment.

This patch looks ready to land (once the other comments are addressed), I only have two of NFC request that can be done pre-commit for readability.



================
Comment at: llvm/lib/Target/PowerPC/Disassembler/PPCDisassembler.cpp:336
                                              raw_ostream &CS) const {
+  auto *ReadFunc = IsLittleEndian ? support::endian::read32le
+                                  : support::endian::read32be;
----------------
nit: Is it possible to use that actually type here instead of `auto`? I get that its hard to define because it's a template within a template but as far as i can tell in this use it would be `uint32_t (*ReadFunc) = ` this would make the code a more readable I think, selecting which function to use base on the target is more clear this way to me with this way.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:82
+  let Inst{8-10} = 0;
+  let Inst{11} = 0;
+  let Inst{12-13} = 0;
----------------
should this be `let Inst{11} = PCRel;` to match the definition in `MLS_DForm_R_SI34_RTA5` since `PI` sets it to zero and only `isPCRel` sets it to 1


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

https://reviews.llvm.org/D72569