[PATCH] D60486: [AArch64] Add support for MTE intrinsics

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td:692
+def int_aarch64_irg   : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty, llvm_i64_ty],
+    [IntrInaccessibleMemOnly]>;
+def int_aarch64_addg  : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty, llvm_i64_ty],
----------------
chill wrote:
> eugenis wrote:
> > Sorry for such a late question.
> > Why is IRG not IntrNoMem? Does this model the fact that it updates the internal state of the RNG in the CPU?
> > 
> > It is not covered by tests.
> > 
> `IntrNoMem` would be incorrect, as that means also no side effects whatsoever, and `IRG` potentially has side effects.
> On the other hand it, in fact does not modify memory, so `IntrInaccessibleMemOnly` is pessimistic and I can't see how being pessimistic in this case ensures correctness (the only good reason for being pessimistic IMHO).
> The spot on attribute would be `IntrHasSideEffects`.
Sounds good, I can prepare a change.


================
Comment at: llvm/trunk/test/CodeGen/AArch64/arm64-mte.ll:12
+  ret i32* %3
+;CHECK: irg x0, x0, {{x[0-9]+}}
+}
----------------
chill wrote:
> `IRG` is mentioned here.
What I meant is, there are no tests that notice the change in the intrinsic attribute.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60486