[AArch64] Mark neon.stN intrinsics as writeonly

[nikic]

I found this peculiar comment in EarlyCSE:

llvm-project/llvm/lib/Transforms/Scalar/EarlyCSE.cpp

Lines 1620 to 1624 in 1c78d8d

	// from memory in the exception handler), forget LastStore. Load/store
	// intrinsics will indicate both a read and a write to memory. The target
	// may override this (e.g. so that a store intrinsic does not read from
	// memory, and thus will be treated the same as a regular store for
	// commoning purposes).

Looking back over history, this seems to be referring to the aarch64.neon.stN intrinsics, which are indeed not marked writeonly (though the ldN intrinsics are readonly).

Possibly I'm missing something special about these intrinsics, but I think it is safe to mark them as writeonly.

[llvmbot]

@llvm/pr-subscribers-backend-arm
@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-backend-aarch64

Author: Nikita Popov (nikic)

Changes

I found this peculiar comment in EarlyCSE:

llvm-project/llvm/lib/Transforms/Scalar/EarlyCSE.cpp

Lines 1620 to 1624 in 1c78d8d

	// from memory in the exception handler), forget LastStore. Load/store
	// intrinsics will indicate both a read and a write to memory. The target
	// may override this (e.g. so that a store intrinsic does not read from
	// memory, and thus will be treated the same as a regular store for
	// commoning purposes).

Looking back over history, this seems to be referring to the aarch64.neon.stN intrinsics, which are indeed not marked writeonly (though the ldN intrinsics are readonly).

Possibly I'm missing something special about these intrinsics, but I think it is safe to mark them as writeonly.

---

Full diff: https://github.com/llvm/llvm-project/pull/145289.diff

1 Files Affected:

• (modified) llvm/include/llvm/IR/IntrinsicsAArch64.td (+7-7)

diff --git a/llvm/include/llvm/IR/IntrinsicsAArch64.td b/llvm/include/llvm/IR/IntrinsicsAArch64.td
index 0ec5f5163118e..3606bbe29eb93 100644
--- a/llvm/include/llvm/IR/IntrinsicsAArch64.td
+++ b/llvm/include/llvm/IR/IntrinsicsAArch64.td
@@ -612,7 +612,7 @@ let TargetPrefix = "aarch64" in {  // All intrinsics start with "llvm.aarch64.".
                   [IntrReadMem, IntrArgMemOnly]>;
   class AdvSIMD_1Vec_Store_Lane_Intrinsic
     : DefaultAttrsIntrinsic<[], [llvm_anyvector_ty, llvm_i64_ty, llvm_anyptr_ty],
-                [IntrArgMemOnly, NoCapture<ArgIndex<2>>]>;
+                [IntrWriteMem, IntrArgMemOnly, NoCapture<ArgIndex<2>>]>;
 
   class AdvSIMD_2Vec_Load_Intrinsic
     : DefaultAttrsIntrinsic<[LLVMMatchType<0>, llvm_anyvector_ty],
@@ -626,11 +626,11 @@ let TargetPrefix = "aarch64" in {  // All intrinsics start with "llvm.aarch64.".
   class AdvSIMD_2Vec_Store_Intrinsic
     : DefaultAttrsIntrinsic<[], [llvm_anyvector_ty, LLVMMatchType<0>,
                      llvm_anyptr_ty],
-                [IntrArgMemOnly, NoCapture<ArgIndex<2>>]>;
+                [IntrWriteMem, IntrArgMemOnly, NoCapture<ArgIndex<2>>]>;
   class AdvSIMD_2Vec_Store_Lane_Intrinsic
     : DefaultAttrsIntrinsic<[], [llvm_anyvector_ty, LLVMMatchType<0>,
                  llvm_i64_ty, llvm_anyptr_ty],
-                [IntrArgMemOnly, NoCapture<ArgIndex<3>>]>;
+                [IntrWriteMem, IntrArgMemOnly, NoCapture<ArgIndex<3>>]>;
 
   class AdvSIMD_3Vec_Load_Intrinsic
     : DefaultAttrsIntrinsic<[LLVMMatchType<0>, LLVMMatchType<0>, llvm_anyvector_ty],
@@ -644,12 +644,12 @@ let TargetPrefix = "aarch64" in {  // All intrinsics start with "llvm.aarch64.".
   class AdvSIMD_3Vec_Store_Intrinsic
     : DefaultAttrsIntrinsic<[], [llvm_anyvector_ty, LLVMMatchType<0>,
                      LLVMMatchType<0>, llvm_anyptr_ty],
-                [IntrArgMemOnly, NoCapture<ArgIndex<3>>]>;
+                [IntrWriteMem, IntrArgMemOnly, NoCapture<ArgIndex<3>>]>;
   class AdvSIMD_3Vec_Store_Lane_Intrinsic
     : DefaultAttrsIntrinsic<[], [llvm_anyvector_ty,
                  LLVMMatchType<0>, LLVMMatchType<0>,
                  llvm_i64_ty, llvm_anyptr_ty],
-                [IntrArgMemOnly, NoCapture<ArgIndex<4>>]>;
+                [IntrWriteMem, IntrArgMemOnly, NoCapture<ArgIndex<4>>]>;
 
   class AdvSIMD_4Vec_Load_Intrinsic
     : DefaultAttrsIntrinsic<[LLVMMatchType<0>, LLVMMatchType<0>,
@@ -667,12 +667,12 @@ let TargetPrefix = "aarch64" in {  // All intrinsics start with "llvm.aarch64.".
     : DefaultAttrsIntrinsic<[], [llvm_anyvector_ty, LLVMMatchType<0>,
                  LLVMMatchType<0>, LLVMMatchType<0>,
                  llvm_anyptr_ty],
-                [IntrArgMemOnly, NoCapture<ArgIndex<4>>]>;
+                [IntrWriteMem, IntrArgMemOnly, NoCapture<ArgIndex<4>>]>;
   class AdvSIMD_4Vec_Store_Lane_Intrinsic
     : DefaultAttrsIntrinsic<[], [llvm_anyvector_ty, LLVMMatchType<0>,
                  LLVMMatchType<0>, LLVMMatchType<0>,
                  llvm_i64_ty, llvm_anyptr_ty],
-                [IntrArgMemOnly, NoCapture<ArgIndex<5>>]>;
+                [IntrWriteMem, IntrArgMemOnly, NoCapture<ArgIndex<5>>]>;
 }
 
 // Memory ops

[davemgreen]

I don't know of a reason why this would be a problem. The MVE intrinsics already specify IntrWriteMem. Should we add it to the similar Arm neon intrinsics too, either here or in a followup?

[efriedma-quic]

Seems fine; the intrinsics are supposed to have the same memory semantics as normal IR operations. (We currently don't provide "volatile" variants. You can use inline asm if you really need that for some strange reason.)

Please add a regression test of some sort to track that we've added this marking.

[nikic]

I don't know of a reason why this would be a problem. The MVE intrinsics already specify IntrWriteMem. Should we add it to the similar Arm neon intrinsics too, either here or in a followup?

I've added the Arm variants now as well.

[davemgreen]

LGTM