[PATCH] D150388: [CodeGen]Allow targets to use target specific COPY instructions for live range splitting

Yashwant Singh via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 23 00:05:32 PDT 2023

yassingh added inline comments.

Comment at: llvm/lib/Target/Mips/MipsSEInstrInfo.cpp:229
   // from copyPhysReg function.
-  if (isReadOrWriteToDSPReg(MI, isDSPControlWrite)) {
-    if (!MI.getOperand(1).isImm() || MI.getOperand(1).getImm() != (1 << 4))
-      return std::nullopt;
-    else if (isDSPControlWrite) {
-      return DestSourcePair{MI.getOperand(2), MI.getOperand(0)};
-    } else {
-      return DestSourcePair{MI.getOperand(0), MI.getOperand(2)};
-    }
-  } else if (MI.isMoveReg() || isORCopyInst(MI)) {
+  if (isReadOrWriteToDSPReg(MI, isDSPControlWrite))
+    return std::nullopt;
sdardis wrote:
> I believe the crash associated with the most immediate version of this patch was due to the definition of those instructions which take an immediate mask to specify which fields to read or write as the immediate operand is the place of the expected source operand.
> This `if` branch can be removed with some changes I think.
> The MIPS `rddsp` and `wrdsp` instructions can read/write either fields or the entire `DSPCond` register, provided the instruction flag of 'isMoveReg' is set to zero for (RD|WR)DSP instructions. As those instructions take the `DSPCond`s subregisters as implicit operands, I don't think those instructions really match the expectations of the `isCopy` hook in their current form.
> I would suggest removing `isMoveReg = 1` from `wrdsp` and `rddsp` so they aren't considered simple copy-like instructions for the moment.
Thanks for the feedback @sdardis. I have removed "isMoveReg=1" from both instructions as you suggested. D151181

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list