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

Tue Jul 18 02:21:29 PDT 2023

khei4 added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1616
+    Builder.SetInsertPoint(LastUser->getParent(), ++LastUser->getIterator());
+    Builder.CreateLifetimeEnd(SrcAlloca, AllocaSize);
+
----------------
hoy wrote:
> Hi, it looks like this resulted in a case where a liftimeEnd was inserted to the end of a block and invalidating the terminator convention:
> 
> 
> ```
> 
> invoke fastcc void @_ZN5folly3f146detail11F14BasicMapINS1_19NodeContainerPolicyIN8facebook8infrasec13authorization8IdentityEKSt10shared_ptrIKNS_10F14NodeMapINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEENS_10F14NodeSetISF_NS_23HeterogeneousAccessHashISF_vEENS_26HeterogeneousAccessEqualToISF_vEESaISF_EEESI_SK_SaISt4pairIKSF_SM_EEEEEvvvEEE13initialInsertIN6ranges14basic_iteratorINSY_14adaptor_cursorINSZ_INSY_9join_viewINSY_14transform_viewINSY_8ref_viewISt6vectorINS5_9Flattener18FlattenedGroupDataESaIS16_EEEEMS16_NS9_IS7_SU_NSH_IS7_vEENSJ_IS7_vEESaISN_IKS7_SU_EEEEEEE6cursorILb0EEEEENSY_9move_viewIS1I_E7adaptorILb0EEEEEEEEEvT_S1S_m(ptr noundef nonnull align 8 dereferenceable(24) %0, ptr noundef nonnull byval(%"struct.ranges::basic_iterator.638") align 8 %5, ptr noundef nonnull byval(%"struct.ranges::basic_iterator.638") align 8 %3, i64 noundef 0)
>           to label %22 unwind label %20, !dbg !56200
>   call void @llvm.lifetime.end.p0(i64 40, ptr %5), !dbg !54832
> ```
> 
> This in turns caused a compiler ICE:
> 
> 
> ```
> clang:      llvm-project/llvm/include/llvm/Support/Casting.h:662: decltype(auto) llvm::dyn_cast(From *) [To = llvm::InvokeInst, From = llvm::Instruction]: Assertion `detail::isPresent(Val) && "dyn_cast on a non-existent value"' failed.
> PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
> Stack dump:
> 0.      Program arguments: /home/hoy/install/llvm-chk/bin/clang @clang.rsp
>  #0 0x00007f85fd06e248 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int)      llvm-project/llvm/lib/Support/Unix/Signals.inc:602:13
>  #1 0x00007f85fd06c4c0 llvm::sys::RunSignalHandlers()      llvm-project/llvm/lib/Support/Signals.cpp:105:18
>  #2 0x00007f85fd06e8ed SignalHandler(int)      llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
>  #3 0x00007f86011cccf0 __restore_rt (/lib64/libpthread.so.0+0x12cf0)
>  #4 0x00007f85fc1e0acf raise (/lib64/libc.so.6+0x4eacf)
>  #5 0x00007f85fc1b3ea5 abort (/lib64/libc.so.6+0x21ea5)
>  #6 0x00007f85fc1b3d79 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d79)
>  #7 0x00007f85fc1d9426 (/lib64/libc.so.6+0x47426)
>  #8 0x00007f85fe6641e1 decltype(auto) llvm::dyn_cast<llvm::InvokeInst, llvm::Instruction>(llvm::Instruction*)      llvm-project/llvm/include/llvm/Support/Casting.h:663:10
>  #9 0x00007f85fe664d0f markAliveBlocks(llvm::Function&, llvm::SmallPtrSetImpl<llvm::BasicBlock*>&, llvm::DomTreeUpdater*)      llvm-project/llvm/lib/Transforms/Utils/Local.cpp:2514:15
> #10 0x00007f85fe6645eb llvm::removeUnreachableBlocks(llvm::Function&, llvm::DomTreeUpdater*, llvm::MemorySSAUpdater*)      llvm-project/llvm/lib/Transforms/Utils/Local.cpp:2663:8
> #11 0x00007f85fed64542 simplifyFunctionCFGImpl(llvm::Function&, llvm::TargetTransformInfo const&, llvm::DominatorTree*, llvm::SimplifyCFGOptions const&)      llvm-project/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp:0:22
> #12 0x00007f85fed64542 simplifyFunctionCFG(llvm::Function&, llvm::TargetTransformInfo const&, llvm::DominatorTree*, llvm::SimplifyCFGOptions const&)      llvm-project/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp:301:18
> #13 0x00007f85fed64357 llvm::SimplifyCFGPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&)      llvm-project/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp:363:7
> ```
> 
> Reverting the patch clears the error. 
@hoy Thank you for the report with helpful info! Yeah, I forgot to port that handling [[ https://reviews.llvm.org/D140089 | the original patch ]] does. Sorry for being late, I assume this is not so difficult but I'll revert it first.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153453