[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics
Dave Green via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 9 08:34:22 PST 2021
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
More information about the cfe-commits
mailing list