[PATCH] D150967: precommit test for memcpy removal for noalias, readonly attributed arguments

[Kohei Asano via Phabricator via llvm-commits]

khei4 added inline comments.


================
Comment at: llvm/test/Transforms/MemCpyOpt/memcpy.ll:398
+declare void @fnr(ptr noalias readonly)
+define noundef i32 @noalias_readonly_i8(ptr align 4 noalias readonly %val) {
+; CHECK-LABEL: @noalias_readonly_i8(
----------------
nikic wrote:
> Drop noundef, it's not relevant. Can also change return type to void. I also don't think `noalias readonly` on the function argument matter, only `align 4`. The `noalias readonly` are relevant on the call only.
> Drop noundef, it's not relevant. Can also change return type to void. 

Thanks! They seemed correct!

> I also don't think noalias readonly on the function argument matter, only align 4. The noalias readonly are relevant on the call only.

I worry about the case like

```
#include <thread>

void ro(int &d) { d; }

void w(int &d) { d = 13; }

int main() {
  int *src, *dest1, *dest2;
  *src = 42;
  std::memcpy(dest1, src, sizeof dest1);
  std::thread ro_t1(ro, dest1);
  std::memcpy(dest2, src, sizeof dest2);
  std::thread w_t(w, dest2);
  std::thread ro_t2(ro, dest2);
}
```
(although I'm not sure `noalias` can be actually attributed for this specific case)

It seems like the transformation would break the `noalias` property of the callee parameter, because memcpy might be used to introduce the actual `noalias` property.  


================
Comment at: llvm/test/Transforms/MemCpyOpt/memcpy.ll:409
+  call void @llvm.memcpy.p0.p0.i64(ptr align 4 %val1, ptr align 4 %val, i64 1, i1 false)
+  %0 = call noundef i32 @fnr(ptr align 4 noalias readonly %val1)
+  ret i32 %0
----------------
nikic wrote:
> Also needs `nocapture`, otherwise the address may be significant.
Thanks! Yeah, removing memcpy by only `noalias` and `readonly` might broke capturing behavior! 
Also I noticed `readnone` doesn't need `nocapture`, so I'll add a test for that case :)



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

https://reviews.llvm.org/D150967