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

Mark Mendell via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 8 12:56:36 PDT 2021


mmendell 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:
> 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
Yes, you are right.   I will pass in LI


================
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
----------------
Meinersbur wrote:
> 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.
If I make %PREV a function pointer, then the cast isn't done: [[ https://godbolt.org/z/WW1ns51jn  | compiler explorer]]
I can remove 2 lines (defn and use of %r2) and still get it to fail.  Anything else, and it doesn't generate the sroa_cast/sroa_cast2


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

https://reviews.llvm.org/D103254



More information about the llvm-commits mailing list