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

[Yashwant Singh via Phabricator via llvm-commits]

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150388