This is an archive of the discontinued LLVM Phabricator instance.

[SCEVExpander] [PowerPC] clear scev rewriter before deleting instructions.
ClosedPublic

Authored by shchenz on Aug 3 2020, 5:59 AM.

Details

Summary

In PowerPC pass PPCLoopInstrFormPrep, we forget to clear scev rewriter SCEVE before we call RecursivelyDeleteTriviallyDeadInstructions, this will cause some our downstream tests fail if we turn on assertion.
The failure is like:

While deleting: i32* %uglygep16341635
An asserting value handle still pointed to this value!
UNREACHABLE executed at ../lib/IR/Value.cpp:1012!

 #9 0x0000000012902514 llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) 
#10 0x000000001218b458 llvm::ValueHandleBase::ValueIsDeleted(llvm::Value*) 
#11 0x000000001218b95c llvm::Value::~Value() 
#12 0x00000000120e3580 llvm::Instruction::~Instruction() 
#14 0x00000000120e37dc llvm::Instruction::eraseFromParent() 
#15 0x0000000012a2c1e8 llvm::RecursivelyDeleteTriviallyDeadInstructions(llvm::SmallVectorImpl<llvm::WeakTrackingVH>&, llvm::TargetLibraryInfo const*, llvm::MemorySSAUpdater*
, std::function<void (llvm::Value*)>) 
#16 0x0000000012a2bad4 llvm::RecursivelyDeleteTriviallyDeadInstructions(llvm::Value*, llvm::TargetLibraryInfo const*, llvm::MemorySSAUpdater*, std::function<void (llvm::Valu
e*)>)
#17 0x00000000112b4dc0 (anonymous namespace)::PPCLoopInstrFormPrep::rewriteLoadStores(llvm::Loop*, (anonymous namespace)::Bucket&, llvm::SmallSet<llvm::BasicBlock*, 16u, std
::less<llvm::BasicBlock*> >&, (anonymous namespace)::InstrForm)
#18 0x00000000112b1b48 (anonymous namespace)::PPCLoopInstrFormPrep::runOnFunction(llvm::Function&)
#19 0x00000000121204ec llvm::FPPassManager::runOnFunction(llvm::Function&) 
#20 0x0000000012128650 llvm::FPPassManager::runOnModule(llvm::Module&)
#21 0x0000000012120d84 llvm::legacy::PassManagerImpl::run(llvm::Module&)
#22 0x0000000012128c5c llvm::legacy::PassManager::run(llvm::Module&)
#23 0x0000000012c22634 clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clan

According to https://reviews.llvm.org/rL109478, before deleting instructions, scev expander's client should clear the containers which contain the values associating with AssertingVH handles.

IndVarSimplify and LSR should be ok. PowerPC PPCLoopInstrFormPrep misses this. Seems HardwareLoops also misses this?

The downstream test is very big and I failed to narrow down it to a small case. Since this patch is quite straightforward, I left it without test. Hope this is ok.

Diff Detail

Event Timeline

shchenz created this revision.Aug 3 2020, 5:59 AM
shchenz requested review of this revision.Aug 3 2020, 5:59 AM
lebedev.ri accepted this revision.Aug 3 2020, 6:07 AM

Not sure this needs review.

This revision is now accepted and ready to land.Aug 3 2020, 6:07 AM