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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/Disassembler/PPCDisassembler.cpp:197
+  if (Imm != 0)
+    return MCDisassembler::Fail;
+  Inst.addOperand(MCOperand::createImm(Imm));
----------------
Yi-Hong.Lyu wrote:
> Two things:
> 
> 1. There is no test that execute this line. Tests doesn't have enough coverage in this patch.
> 2. For `paddi 1, 2, 8589934591, 1`, it is disassemblable but invalid on hardware. I imagine we should use `SoftFail` instead of `Fail` here. Please see https://llvm.org/doxygen/classllvm_1_1MCDisassembler.html#a8eb822283e8f3200ca4b2a1ba0174e6a 
I am not sure what your intent here is. The instruction as you wrote it is **illegal**. It will cause a `SIGILL` not some kind of undefined behaviour or anything like that.
What would be the advantage of disassembling object code into invalid instruction forms?


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

https://reviews.llvm.org/D72569