[PATCH] D86020: [MemCpyOptimizer] Optimize passing byref function arguments down the stack

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D86020#2222103 <https://reviews.llvm.org/D86020#2222103>, @atrosinenko wrote:

> @rjmccall
>
> Maybe I overestimated similarity of `byval` and recently introduced `byref`... Looks like some aliasing restrictions are not mentioned in LLVM Language Reference <https://llvm.org/docs/LangRef.html#parameter-attributes>. For example, the only way for Clang to emit the `byref` attribute to LLVM IR I know about is via `ABIArgInfo` with `Kind == IndirectAliased` introduced in D79744: clang: Use byref for aggregate kernel arguments <https://reviews.llvm.org/D79744> - and it has a note that "The object is known to not be through any other references for the duration of the call, and the callee must not itself modify the object". This seems to be necessary for correctness of this transformation. If LLVM Language Reference does not mention such restrictions intentionally, then it may be clang that should be responsible for such optimizations.

I think this lost the word written or stored



================
Comment at: llvm/test/Transforms/MemCpyOpt/byref-memcpy.ll:8
+; A caller of this function has to arrange everything for the pointer it passes
+; to be usable with byref attrbiute anyway, so just pass the pointer through.
+define void @function_with_byref_arg(%struct.S* byref(%struct.S) align 4 %0) {
----------------
Typo attrbiute


================
Comment at: llvm/test/Transforms/MemCpyOpt/byref-memcpy.ll:55
+; CHECK-NEXT:    [[V2:%.+]] = bitcast %struct.S* [[V0]] to i8*
+; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i16(i8* align 4 [[V1]], i8* align 2 [[V2]], i16 16, i1 false)
+; CHECK-NEXT:    call void @leaf(%struct.S* byref(%struct.S) align 4 [[ALLOCA]])
----------------
atrosinenko wrote:
> Interestingly, if I change the alignment of the second argument from 2 back to 4, then memcpy() is successfully dropped (as if everything has big-enough alignment) while it seemingly shouldn't be. Is it just due to the code being ill-formed?
I'm not sure I follow, if you change it to 4 it is big enough?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86020