[PATCH] D136758: [ARM] Use DefaultAttrsIntrinsics

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 26 10:55:26 PDT 2022


dmgreen added reviewers: efriedma, simon_tatham, miyuki.
dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

Thanks for doing this. I have left some comments but as far as I understand the changes LGTM.



================
Comment at: llvm/include/llvm/IR/IntrinsicsARM.td:318
                        Intrinsic<[], [llvm_i32_ty], []>;
 def int_arm_vcvtr     : Intrinsic<[llvm_float_ty], [llvm_anyfloat_ty],
                                   [IntrNoMem]>;
----------------
Given [WillReturn, NoSync, NoFree and NoCallback], these FP intrinsics sounds OK.


================
Comment at: llvm/include/llvm/IR/IntrinsicsARM.td:500
 
 let IntrProperties = [IntrNoMem, Commutative] in {
 
----------------
This will continue to work? It looks like the default flags are added through a different means.


================
Comment at: llvm/include/llvm/IR/IntrinsicsARM.td:1398
 
+// TODO: Add applicable default attributes.
 multiclass CDEGPRIntrinsics<list<LLVMType> args> {
----------------
CDE should be fine - there are limits to what a valid CDE instruction can do which should mean the default flags are all OK.


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

https://reviews.llvm.org/D136758



More information about the llvm-commits mailing list