[PATCH] D88044: [llvm-exegesis][PowerPC] Add more register classes

Jinsong Ji via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 30 09:20:40 PST 2020


jsji added a comment.

Thanks @qiucf for looking into this.

> Besides, I found llvm-exegesis -mode=latency -opcode-name=XSTDIVDP complains 'illegal instruction' on ppc64le-pwr9, which looks strange. Is that because we did not model CR well in this tool?

It depends. We will generate *random* instructions to form the pattern, so you should try to dump the object to see what is causing `illegal instruction`.

But on the other hand, yes, as I mentioned , this is an incremental patch, it is far from complete.
So some test may still invalid and causing failures.

We will work to fix them in following patches.



================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:22
   let DecoderMethod = "decodeSImmOperand<16>";
+  let OperandType = "OPERAND_IMMEDIATE";
 }
----------------
qiucf wrote:
> So, we will mark `OPERAND_REGISTER` in future patches?
We may, but not necessary, it depends on whether we want to update `OperandType` for all td files or just update when it matters.

This patch only update for imm as it is required and used in llvm-exegesis.


================
Comment at: llvm/tools/llvm-exegesis/lib/PowerPC/Target.cpp:111
+            MCInstBuilder(PPC::MTVRD).addReg(Reg).addReg(ScratchImmReg)};
+  if (PPC::VSRCRegClass.contains(Reg))
+    return {loadImmediate(ScratchImmReg, 64, Value),
----------------
qiucf wrote:
> I applied this patch and tested `XSDIVDP`, still got 2 lines of warning `setRegTo is not implemented, results will be unreliable` (before this patch, that is six). Is that from `RM` and can be ignored?
Yes, you are right. This patch implemented *more* RC, but not a complete one. We will add more RC in later patch.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88044



More information about the llvm-commits mailing list