[PATCH] D103254: Preserve more MD_mem_parallel_loop_access and MD_access_group in SROA

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:2478
+    cast<Instruction>(V)->copyMetadata(
+        NewAI, {LLVMContext::MD_mem_parallel_loop_access,
+                LLVMContext::MD_access_group});
----------------
Meinersbur wrote:
> `NewAI` is the alloca instruction; it does not have parallel loop access or access group metadata which only apply to memory accesses.
This one might have made sense


================
Comment at: llvm/test/Transforms/SROA/mem-par-metadata-sroa-cast.ll:8-9
+; CHECK: entry:
+; CHECK: load i32, i32* {{.*}}, !llvm.access.group [[DISTINCT:![0-9]*]]
+; CHECK: load i32, i32* {{.*}}, !llvm.access.group [[DISTINCT]]
+; CHECK: ret void
----------------
mmendell wrote:
> Meinersbur wrote:
> > This tests the `presplitLoadsAndStores` case. Shouldn't it also have split the store?
> The store is replaced with:
>   %0 = bitcast i32 %X371 to float
>   %1 = bitcast i32 %X373 to float
> Should I add that to the LIT test?  The important part was the loss of the llvm.access.group from the store
The bitcasts belong to the load. I guess SROA determined that the store is dead (stack memory before returning) and removed it. Not an indication of a good test case.

I suggest to make `%PREV` a function pointer so this does not happen. Attach a separate `!llvm.access.group !1` to the store and test it as well. Most other instructions in this function seems to be irrelevant.


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

https://reviews.llvm.org/D103254