[PATCH] D110049: [AMDGPU] Correctly merge alias.scope and noalias metadata for memops

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/lower-lds-struct-aa-memcpy.ll:20
+; CHECK-LABEL: @test
+; CHECK: store i8 3, i8 addrspace(3)* %0, align 4, !alias.scope !0, !noalias !3
+; CHECK: tail call void @llvm.memcpy.p3i8.p3i8.i64(i8 addrspace(3)* noundef align 1 dereferenceable(3) %2, i8 addrspace(3)* noundef align 1 dereferenceable(3) %1, i64 3, i1 false), !alias.scope !6, !noalias !7
----------------
It is unclear from the test what are these !0, !3 etc values. Could you include it in the checks?


================
Comment at: llvm/test/CodeGen/AMDGPU/lower-lds-struct-aa-memcpy.ll:29
+  store i8 3, i8 addrspace(3)* getelementptr inbounds (%vec_type, %vec_type addrspace(3)* @_f1, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0), align 1
+  tail call void @llvm.memcpy.p3i8.p3i8.i64(i8 addrspace(3)* noundef align 1 dereferenceable(3) getelementptr inbounds (%vec_type, %vec_type addrspace(3)* @_f2, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0), i8 addrspace(3)* noundef align 1 dereferenceable(3) getelementptr inbounds (%vec_type, %vec_type addrspace(3)* @_f1, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0), i64 3, i1 false)
+  %0 = load i8, i8 addrspace(3)* getelementptr inbounds (%vec_type, %vec_type addrspace(3)* @_f2, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0), align 1
----------------
The original memcpy did not have any AA, we are merging AA produced by the same pass.

As I understand the problem is there is a single AA for the whole instruction, not for an operand. Clearly concat is not correct if we pass 2 different LDS pointers into a same call, but passing AA for only the first operand is not correct either, please disregard previous comment with respect to keeping the old AA value.

Using the intersect shall be fine, although on practice it will produce an empty list if pointers are to different fields. With that in mind the patch itself is correct, but please add metadata checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110049