[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

[Dave Green via Phabricator via llvm-commits]

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