dmgreen added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495
   let DecoderNamespace = "Fallback";
+  let Defs = [NZCV];
 }
----------------
SjoerdMeijer wrote:
> Do all MRS instructions do this?
No, but some do and it's not obvious which ones do and don't. I think it should 
be reasonable to always def NZCV here, even if they are just dead. It should be 
very rare that it would be beneficial for NZCV to be live across a MRS 
instruction.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:1270
+// MRS from CodeGen.
+def AArch64mrs      : SDNode<"AArch64ISD::MRS",
+                             SDTypeProfile<1, 1, [SDTCisVT<0, i64>, 
SDTCisVT<1, i32>]>,
----------------
These are usually put somewhere else in the aarch64 backend, and it might be 
worth keeping it with the others.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:1274
+def : Pat<(AArch64mrs imm:$id),
+          (MRS imm:$id)>;
+
----------------
SjoerdMeijer wrote:
> Nit: can be on the same line.
I always prefer Pat's to have input and output lines separate, for what it's 
worth. It makes them more easily readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98264

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to