[PATCH] D153453: [MemCpyOpt] implement single BB stack-move optimization which unify the static unescaped allocas

Mon Aug 7 23:18:26 PDT 2023

khei4 added a comment.

> I looked into this a bit, and I believe the problem here is that we're not adding MSSA MemoryAccesses for the newly inserted lifetime intrinsics. The actual intrinsic placement transform itself is correct, it's just that following MSSA queries fail to see them and perform incorrect transforms as a result.

@nikic 
Thanks! You are right. I created MemoryAccess for newly created lifetime markers.

> If we limit to alloca with at most one pair of start/end we just need to pick which start/end out of up 4 markers to keep using Domination.
>
> e.g, if either src or dst has no markers, we strip markers from src
> if src.start dominates dst.start then keep src.start
> if dst.start dominates src.start then keep dst.start
> if neither, don't perform transformation
>
> similar with ends and postdominate.

@vitalybuka

Thank you! I missed this. Although I'm not pretty sure about the lifetime-related optimizations e.g. stack-coloring, I begin to feel like if the transformation is possible(with any interleaving lifetime), then the transformed one(single shrink-wrapped lifetime) may be not worse than the original one.

================
Comment at: llvm/test/Transforms/MemCpyOpt/lifetime-missing.ll:22
-; CHECK-NEXT:    [[AGG_TMP3_SROA_35_128_SROA_IDX:%.*]] = getelementptr i8, ptr [[AGG_TMP3_SROA_35]], i64 4
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr inttoptr (i64 4 to ptr), ptr [[AGG_TMP3_SROA_35_128_SROA_IDX]], i64 1, i1 false)
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr null, ptr [[AGG_TMP3_SROA_35_128_SROA_IDX]], i64 1, i1 false)
----------------
I thought This memcpy removal is because of undef value copying, but if so original example should be also removed. The current MemoryAccess creation might not be complete.

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

https://reviews.llvm.org/D153453