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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:364
+  if (HasRandGen)
+    Builder.defineMacro("__ARM_FEATURE_RNG", "1");
+
----------------
This needs a clang preprocessor test: both a check for its presence and absence.
Have a look/grep where these others predefined macros are tested.


================
Comment at: clang/test/CodeGen/arm_acle.c:908
+#if __ARM_64BIT_STATE && defined(__ARM_FEATURE_RNG)
+// AArch64-v8_3-LABEL: @test_rndr(
+// AArch64-v8_3-NEXT:  entry:
----------------
Not sure if I am surprised that this works.... This is (re)using tag `AArch64-v8_3` and for the other tests that use this and don't have RNG set, I was expecting FileCheck to complain about this, not sure if I am missing something though.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:610
 def AArch64tbl : SDNode<"AArch64ISD::TBL", SDT_AArch64TBL>;
-
+// MRS from CodeGen.
+def AArch64mrs : SDNode<"AArch64ISD::MRS",
----------------
Nit: for this comment to be informative, please expand it, or just remove it if there's not much to say about it.


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

https://reviews.llvm.org/D98264